2.96 KB, patch
|Details | Diff | Splinter Review|
Created attachment 8827426 [details] [diff] [review] remove-minus-zero-from-range-for-int32-modulo.patch A truncated integer division/modulo range cannot contain negative zero. The current patch changes the range of idiv/imod in ::truncate(), which is not allowed. It should set the correct range in ::computeRange(), but it cannot do that because the range changes when idiv/imod is truncated (e.g. no negative zero). Excerpt of an IRC discussion: 16:38 <h4writer> smvv: range should only get updated in computeRange. (MMod::truncate happens later as a result we shouldn't change the range anymore!) 16:39 <smvv> h4writer: the problem is that I can only reduce the range when I know it is truncated. Kind of a problem given that that is only known after computing the range 16:40 <smvv> h4writer: (a / b)|0 cannot produce -0 but I cannot set that in computeRange because I will only know that it is truncated in ::truncate() (which is after computeRange()). Same thing for (a % b)|0 16:44 <smvv> h4writer: what would you do to check if the result of mmod or mdiv is truncated, during computeRange()? 16:44 <h4writer> smvv: interesting. We do have some inefficiencies here. I don't think with the current code it would be possible. We have a similar issue with eagerly marking ranges wrapAroundToInt32 when it still can get truncated. We might want to have a RA pass after truncation where we can incorporate that information. I.e. the truncation you are mentioning and not having to do the wrapAroundToInt32 16:45 <h4writer> (It is not always know at that time yet. Sometimes it is (isTruncated())
First of all: (a / b) can produce -0. c|0 cannot produce -0. So, the answer is that the range of (a / b) should keep its -0, even after the ::truncate() call. The ::truncate() call can specialize the flags used during the lowering & codegen. (for example specialization_ = MIRType::Int32) What are you trying to achieve by removing -0 from the Range?