Closed Bug 1246229 Opened 4 years ago Closed 4 years ago

Repeated bailout & recompilation caused by unboxing of unused loop variables


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox47 --- fixed


(Reporter: nbp, Assigned: nbp)




(1 file)

Currently, in our test suite the test name jit-test/tests/basic/testTypedArrayInit.js is slow, and is made even slower by Bug 1242462, as we allow to OSR multiple times even after a bailout.

Here is likely what happens:
  - We enter the loop a first time.
  - We decide to compile it while we are at the loop entry.
    - When see that one of the value of the loop is unused and replace it by a Magic(JS_OPTIMIZED_OUT)
  - We OSR into it with no issue.
  - We bailout from the loop.
  - We OSR again in the loop, and fail because we cannot unbox the Magic to an Object.

I suggest that if we replace a Phi node in the loop pre-header, we do the same for the OSR block.
The MTypeBarrier and the MUnbox of the OSR block are artificially added as a
way to ensure that we have the same inputs as the non-osr entry.  These
guards are only useful if there is anything which relies on them after.

As all the guards added in the OSR block are part of the data-flow, we can
just ignore the "isGuard" flag, and only rely on the uses of the instruction
to discard or not the MTypeBarrier and the MUnbox.
Attachment #8717026 - Flags: review?(hv1989)
Comment on attachment 8717026 [details] [diff] [review]
Enable DCE to remove OSR guards if their values are optimized-out.

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

::: js/src/jit/IonAnalysis.cpp
@@ +950,5 @@
>  bool
>  js::jit::DeadIfUnused(const MDefinition* def)
>  {
> +    return !def->isEffectful() &&
> +           !(def->isGuard() && def->block() != def->block()->graph().osrBlock()) &&

Can you change this to:
(!def->isGuard() || def->block() == def->block()->graph().osrBlock())
Attachment #8717026 - Flags: review?(hv1989) → review+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.