891055 - Truncate intish operations if their uses are truncated

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Niko Matsakis [:nmatsakis]]

Right now, if you have code like `(a+b)|0`, where `a` and `b` are integers, we will still bailout if the result overflows, although this is not necessary.

Comment 1

[Niko Matsakis [:nmatsakis]]

Attachment: truncate intish operations if their uses are truncated

Comment 2

[Niko Matsakis [:nmatsakis]]

Correction: `(a+b)|0` probably works today, but `(a/b)|0` does not.

Comment 3

[Niko Matsakis [:nmatsakis]]

Related: bug 866670

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 772249 [details] [diff] [review]
truncate intish operations if their uses are truncated

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

::: js/src/ion/RangeAnalysis.cpp
@@ +1756,5 @@
> +            hasNoRoundingErrors = r && !r->hasRoundingErrors();
> +
> +            // To be truncated, must either be intish or have no
> +            // rounding errors.
> +            if (!isIntishOp && !hasNoRoundingErrors)

Considering that this optimization should only help a/b, this is a lot of new code which is mostly redundant with how the range analysis already works.

Ideally, this could be done by adding an MDiv::computeRange able to indicate that for no integers will a/b produce a value with rounding errors, but that doesn't seem possible with the current representation as a/b can be infinity.

Instead, can you just tailor a special case here for integer division, with a comment explaining what's going on in this specific case rather than the obfuscating intish stuff?

Comment 5

[Niko Matsakis [:nmatsakis]]

Attachment: truncate div operations if their uses are truncated

Comment 6

[Brian Hackett [Laid off!]]

Comment on attachment 773163 [details] [diff] [review]
truncate div operations if their uses are truncated

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

::: js/src/ion/RangeAnalysis.cpp
@@ +1726,4 @@
>                default:;
>              }
>  
> +            // Set truncated flag if either (1) range analysis ensure

Maybe fix the existing grammar issue.

@@ +1727,5 @@
>              }
>  
> +            // Set truncated flag if either (1) range analysis ensure
> +            // that it has no rounding errors and no fractional part
> +            // or (2) this is integer division, which has no range.

This comment should describe why the special case is needed: the result of integer division can be infinite, but cannot be a non-infinite double large enough to have rounding errors, and our representation of ranges cannot capture this property.

@@ +1732,4 @@
>              const Range *r = iter->range();
> +            bool hasNoRoundingErrors = r && !r->hasRoundingErrors();
> +            bool isIntegerDiv =
> +                iter->op() == MDefinition::Op_Div &&

iter->isDiv()

@@ +1732,5 @@
>              const Range *r = iter->range();
> +            bool hasNoRoundingErrors = r && !r->hasRoundingErrors();
> +            bool isIntegerDiv =
> +                iter->op() == MDefinition::Op_Div &&
> +                iter->toBinaryArithInstruction()->specialization() == MIRType_Int32;

iter->toDiv()