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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that triggers the issue

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.

Comment 1

[Marty Rosenberg [:mjrosenb]]

Attachment: /home/mrosenberg/patches/dontHoist-r0.patch

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.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

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;
}

Comment 3

[Marty Rosenberg [:mjrosenb]]

landed: http://hg.mozilla.org/integration/mozilla-inbound/rev/ae6f5e7ddef3

Comment 4

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/ae6f5e7ddef3