Closed Bug 891868 Opened 10 years ago Closed 6 months ago

Inline of abs fails if input is not int32 or double

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: nmatsakis, Unassigned)

References

Details

Attachments

(1 file)

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.
Blocks: PJS
Assignee: general → nmatsakis
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)

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

Assignee: nmatsakis → nobody

PJS specific, let's close this.

Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.