Closed Bug 818791 Opened 7 years ago Closed 7 years ago

Loop-hoisting a type barrier from dead code effectively blacklists the loop from running in ion

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: bzbarsky, Unassigned)

Details

Attachments

(2 files)

In the attached testcase, the "if" body inside the loop is dead code.  So we have no type information for it.  But we end up loop-hoisting the getter there, as well as the following type barrier.  And since loop-hoisted things are no longer conditional, that means we take a bailout as soon as we try to run the loop in Ion.  Then we're back doing it under JM and it runs slow; even slower than it would in Ion without LICM.

Bug 669517 might help here long-term, but arguably we should never hoist a type barrier that guarantees bailouts, because that's strictly worse than leaving it where it is, possibly in a rarely taken branch...

Many thanks to Marty for figuring out what was going on here.
Summary: Loop-hoisting a type guard in dead code effectively blacklists the loop from running in ion → Loop-hoisting a type barrier from dead code effectively blacklists the loop from running in ion
this patch is laser-tageted at this bug.  It does nothing intelligent.  A better strategy would probably be to just never hoist out of a block that has never run.  I am unsure if the call empty() does what I want it to do, but it did make the slowness go away.
Attachment #689099 - Flags: review?(dvander)
Comment on attachment 689099 [details] [diff] [review]
/home/mrosenberg/patches/dontHoist-r0.patch

Review of attachment 689099 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/MIR.h
@@ +264,5 @@
>      }
>      void setFlags(uint32 flags) {
>          flags_ |= flags;
>      }
> +    virtual bool reallyDontHoist() const { return false; }

nit: virtual bool neverHoist() const {
    return false;
}
Attachment #689099 - Flags: review?(dvander) → review+
https://hg.mozilla.org/mozilla-central/rev/ae6f5e7ddef3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.