891868 - Inline of abs fails if input is not int32 or double

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Niko Matsakis [:nmatsakis]]

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

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

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

if (getBuildConfiguration().parallelJS)
  testMap();


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.

Comment 1

[Niko Matsakis [:nmatsakis]]

Attachment: Inline as long as we don't predict input is an object.

Comment 2

[Sean Stangl [:sstangl]]

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.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 4

[Jan de Mooij [:jandem]]

PJS specific, let's close this.