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

Status: NEW

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Sander Mathijs van Veen]

Attachment: 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())

Comment 1

[Nicolas B. Pierron [:nbp]]

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?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.