1321439 - LICM moves non-executed overflow math operations, which causes repeated bailouts.

Status: RESOLVED WORKSFORME

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

Description

[Treeherder Bug Filer]

Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=7538323&repo=autoland

https://queue.taskcluster.net/v1/task/N-DCaIw1SWaMxTHkBrI_Xw/runs/0/artifacts/public/logs/live_backing.log

Comment 1

[André Bargull [:anba]]

Same issue as bug 1321199, bug 1318699, bug 1316054, bug 1316053, bug 1315876, bug 1315773, and bug 1315453.

This line in js/src/tests/ecma/shell.js [1] is causing the timeouts:
> return ( (leap == 0) ? 28*msPerDay : 29*msPerDay );

Changing it to:
> return ( (leap == 0 ? 28 : 29)*msPerDay );

improves the time needed to complete the test noticeably (from >9000ms to <200ms for me). Is it worth investigating this issue or should we just change the return expression in the shell.js file?

[1] https://dxr.mozilla.org/mozilla-central/rev/cd4cdcc9ad6c45dad8b8d8c0d40e459db2bca8a1/js/src/tests/ecma/shell.js#595

Comment 2

[Jan de Mooij [:jandem]]

This is annoying. We're hoisting 28*msPerDay out of a loop, it overflows, and we bailout + invalidate the Ion script.

However, this multiplication is never actually executed, so we don't record that it overflows. We end up recompiling the script, bailout again, etc...

Comment 3

[Nicolas B. Pierron [:nbp]]

The bug described in comment 1 and comment 2 still has to be fixed, even if it no longer appear as an intermittent.

Comment 4

[Ted Campbell [:tcampbell]]

Ashley, I think this another form of the bailout bug you are looking at. Maybe we need to avoid hoisting Ion code that has no ICs (and thus never run).

Comment 5

[Ashley Hauck [:khyperia]]

(In reply to Ted Campbell [:tcampbell] from comment #4)
> Ashley, I think this another form of the bailout bug you are looking at.
> Maybe we need to avoid hoisting Ion code that has no ICs (and thus never
> run).

Yep, it is: I debug this at all, but the test case below bails out repeatedly on master (tip? whatever the HG term is), but applying my patch for https://bugzilla.mozilla.org/show_bug.cgi?id=1487022 makes it not bail out at all. That strongly suggests to me that the problem was indeed the "assume int specialization when we haven't seen anything" thing.

Note my patch doesn't change hoisting behavior, but rather simply doesn't specialize never-ran ICs on int.

```
var msPerDay = 86400000;
function f(leap) {
    var sum = 0;
    for (var i = 0; i < 100000; ++i) {
        var x = ( (leap == 0) ? 28*msPerDay : 29*msPerDay );
        sum += Math.log(x);
    }
    return sum;
}
console.log(f(0));
console.log(f(1));
```

Comment 6

[Ashley Hauck [:khyperia]]

(In reply to Ashley Hauck [:khyperia] from comment #5)

> Yep, it is: I debug this at all, [...]

Hmm. Is there no way to edit comments? Meant to say "I didn't debug this at all"... kind of important for the meaning of that comment :P

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

(In reply to Ashley Hauck [:khyperia] from comment #6)

> Hmm. Is there no way to edit comments? Meant to say "I didn't debug this at
> all"... kind of important for the meaning of that comment :P

No, but... https://twitter.com/BugzillaUX/status/1032106489979125760

Comment 8

[André Bargull [:anba]]

No longer reproducible -> WFM.