Inline of abs fails if input is not int32 or double

NEW
Assigned to

Status

()

5 years ago
4 years ago

People

(Reporter: nmatsakis, Assigned: nmatsakis)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 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() {
  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.
(Assignee)

Updated

5 years ago
Blocks: 801869
(Assignee)

Updated

5 years ago
Assignee: general → nmatsakis
(Assignee)

Comment 1

5 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)
(Assignee)

Updated

5 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.