Bug 1688136 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

The bug here is caused by range analysis. Here's a slightly reduced testcase (with `--no-threads --fast-warmup`):
```
function f(a, c) {
    if (c) {
        a++;
    } else {
        a--;
    }
    return (a + a) | 0;
}

with ({}) {}
for (var i = 0; i < 100; i++) {
    f(2147483647, i % 2);
}
```
When we take the `a++` branch, the value of `a` overflows Int32. We attach a stub with a DoubleAddResult for `(a + a)`, and transpile it, producing MIR that looks like this after GVN:
```
17 phi toDouble15:Double add10:Double
22 add phi17:Double phi17:Double [double]
...
24 truncatetoint32 add22:Double

```
Because the only consumer of the add is a truncation, range analysis decides to convert it to Int32. We generate an overflow check, which bails out. We don't make any changes to CacheIR, though, because the existing stub already handles overflow. In release builds without the assertion, this will trigger a bailout/invalidation loop.

The easiest fix here is to mark any `TruncateKind::TruncateAfterBailout` node [here](https://searchfox.org/mozilla-central/rev/d5e98892f9553f9113e69c585308008e681e19c9/js/src/jit/RangeAnalysis.cpp#3191) with `BailoutKind::EagerTruncation`. If we bail out for that node, we will make range analysis more conservative when we recompile. 

It's a little silly that we're doing this optimization at all, since baseline presumably had a good reason for attaching `DoubleAddResult`. It's probably overkill to rip `RangeAnalysis::truncate` out completely, though.
The bug here is caused by range analysis. Here's a slightly reduced testcase (with `--no-threads --fast-warmup`):
```
function f(a, c) {
    if (c) {
        a++;
    } else {
        a--;
    }
    return (a + a) | 0;
}

with ({}) {}
for (var i = 0; i < 100; i++) {
    f(2147483647, i % 2);
}
```
When we take the `a++` branch, the value of `a` overflows Int32. We attach a stub with a DoubleAddResult for `(a + a)`, and transpile it, producing MIR that looks like this after GVN:
```
17 phi toDouble15:Double add10:Double
22 add phi17:Double phi17:Double [double]
...
24 truncatetoint32 add22:Double

```
Because the only consumer of the add is a truncation, range analysis decides to convert it to Int32. We generate an overflow check, which bails out. We don't make any changes to CacheIR, though, because the existing stub already handles overflow. In release builds without the assertion, this will trigger a bailout/invalidation loop.

The easiest fix here is to mark any `TruncateKind::TruncateAfterBailout` node [here](https://searchfox.org/mozilla-central/rev/d5e98892f9553f9113e69c585308008e681e19c9/js/src/jit/RangeAnalysis.cpp#3191) with `BailoutKind::EagerTruncation`. If we bail out for that node, we will make range analysis more conservative when we recompile. 

It's a little silly that we're doing this optimization at all, since baseline presumably had a good reason for attaching `DoubleAddResult`. It's probably overkill to rip `TruncateAfterBailout` out completely, though.

Back to Bug 1688136 Comment 5