1998208 - Division with explicit truncation still emits non-zero remainder check

Status: ASSIGNED

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

Description

[André Bargull [:anba]]

function div(x, y) {
  // Type both arguments as Int32.
  x = x|0;
  y = y|0;

  // Math.max to avoid division by zero and overflow checks.
  y = Math.max(y, 1);

  // Truncating integer division.
  return (x / y)|0;
}

is compiled to:

cdq        
idivl      %ecx
testl      %edx, %edx
jne        .Lbail

Only after commenting out the call to setGuardRangeBailoutsUnchecked in MDiv::collectRangeInfoPreTrunc() (added in bug 1763012) it gets compiled without the unnecessary remainder check:

cdq        
idivl      %ecx

---

But there's a catch: This relies on never seeing a non-Int32 division in CacheIR, i.e. x must always be a multiple of y. If we ever saw a division with a non-zero remainder in CacheIR, we emit a Double-typed division:

                                # LIR=Int32ToDouble
xorpd      %xmm0, %xmm0
cvtsi2sd   %ecx, %xmm0
                                # LIR=Int32ToDouble
xorpd      %xmm1, %xmm1
cvtsi2sd   %eax, %xmm1
                                # LIR=MathD:Div
divsd      %xmm1, %xmm0
                                # LIR=TruncateDToInt32
vcvttsd2si %xmm0, %rax
cmpq       $0x1, %rax
jo         .Lbail
movl       %eax, %eax

Comment 1

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1763012

:iain, since you are the author of the regressor, bug 1763012, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[André Bargull [:anba]]

FWIW explicit truncation also doesn't work for multiplication (bug 1810419).

Comment 3

[Tommy]

Attachment: Bug 1998208 - Move setGuardRangeBailoutsUnchecked after Truncate information is obtained r=anba

Comment 4

[Tommy]

Just uploaded a small patch which fixes the first issue that's mentioned, I suspect it's a similar fix for MMul.

The issue seems to be that the calls to fallible and setGuardRangeBailoutsUnchecked in MDiv::collectRangeInfoPreTrunc occurs before the truncate call in OptimizeMIR which means falliable checks truncate which isn't set yet. Moving it to be after truncate is set seems to fix it.

I'm still looking into the second issue.

Comment 5

[Tommy]

Unfortunately I think this issue is a bit to complex for me, I'll be releasing this one back into the bugzilla lake