IonMonkey: Remove minus zero from int32 modulo range and set truncated div range to int32

NEW

Status

()

Core
JavaScript Engine: JIT
P5
normal
9 months ago
9 months ago

People

(Reporter: Sander Mathijs van Veen, Assigned: Sander Mathijs van Veen)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
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())
(Assignee)

Updated

9 months ago
Assignee: nobody → sandervv
Flags: needinfo?(nicolas.b.pierron)
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?
Flags: needinfo?(nicolas.b.pierron)
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.