Closed Bug 829277 Opened 7 years ago Closed 7 years ago

IonMonkey: truncate analysis can truncate prematurely.

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: mjrosenb, Unassigned)

Details

Attachments

(1 file)

the testcase that nbp gave in bug 825006 can actually trigger errant behavior in ionmonkey as it stands.  Limit the number of instructions that truncation analysis will operate on.
Attachment #700658 - Flags: review?(dvander)
I just instrumented the code a bit, and it looks like it never triggers on kraken or v8, but it does trigger on the test case, so it should fix problems without causing any regressions.
Comment on attachment 700658 [details] [diff] [review]
/home/mjrosenb/patches/fixtruncate-r0.patch

Review of attachment 700658 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this!

::: js/src/ion/EdgeCaseAnalysis.cpp
@@ +92,4 @@
>              continue;
> +        }
> +        if (def->isSub() && def->toSub()->isTruncated()) {
> +            ret = Max(ret, def->toSub()->isTruncated()+1);

nit: spaces around +, here and above.

::: js/src/ion/MIR.cpp
@@ +876,5 @@
>  {
> +    // the add is fallible if range analysis does not say that it is finite, AND
> +    // either the truncation analysis shows that there are non-truncated uses, or
> +    // there are more than 20 operations before it gets truncated
> +    return (!isTruncated() || isTruncated() > 20) && (!range() || !range()->isFinite());

Could you explain where 20 comes from and what it means?
Attachment #700658 - Flags: review?(dvander) → review+
https://hg.mozilla.org/mozilla-central/rev/2983ba3b514f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.