Repeated bailout & recompilation caused by unboxing of unused loop variables

RESOLVED FIXED in Firefox 47

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Trunk
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

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.
Created attachment 8717026 [details] [diff] [review]
Enable DCE to remove OSR guards if their values are optimized-out.

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+

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ad1e7fdc44b3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.