Inline of abs fails if input is not int32 or double

Assigned to



6 years ago
4 years ago


(Reporter: nmatsakis, Assigned: nmatsakis)


Firefox Tracking Flags

(Not tracked)



(1 attachment)



6 years ago
For example, this test case fails parallel execution because the call to Math.abs is not inlined:

load(libdir + "parallelarray-helpers.js");

function testMap() {
    range(0, minItemsTestingThreshold),
    function (i) {
      var tmp = i;
      if (i > 1024)
        tmp += 0.5;
      return Math.abs(tmp);

if (getBuildConfiguration().parallelJS)

Now, with respect to ensuring that parallelizable code executes in parallel, the best fix is to make use of bug 881988 to implement a parallelized version of Math, but in this particular case we ought to be able to inline the computation anyhow.


6 years ago
Blocks: 801869


6 years ago
Assignee: general → nmatsakis

Comment 1

6 years ago
Created attachment 773262 [details] [diff] [review]
Inline as long as we don't predict input is an object.
Attachment #773262 - Flags: review?(sstangl)


6 years ago
Blocks: 891877
Comment on attachment 773262 [details] [diff] [review]
Inline as long as we don't predict input is an object.

Review of attachment 773262 [details] [diff] [review]:

::: js/src/ion/MCallOptimize.cpp
@@ +501,5 @@
> +        input = callInfo.getArg(0);
> +    } else {
> +        types::StackTypeSet *inputTypeSet = callInfo.getArg(0)->resultTypeSet();
> +        if (inputTypeSet->maybeObject())
> +            return InliningStatus_NotInlined;

This will let through Strings and Booleans, on top of Values. The Boolean case can be easily handled in a separate branch via MToInt32.

@@ +511,5 @@
>      }
> +    JS_ASSERT(input->type() == MIRType_Double || input->type() == MIRType_Int32);
> +    MInstruction *ins = MAbs::New(input, input->type());
> +    current->add(ins);

If TI expects the return type to be Int32, it would be a type error to return a double, trippable if the next instruction bails. The path that checks the returnType and applies MToInt32 if necessary should be preserved.
Attachment #773262 - Flags: review?(sstangl)
You need to log in before you can comment on or make changes to this bug.