1148494 - MSimdUnbox should have the Guard flag set when it's fallible

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Dan Gohman [:sunfish]]

Currently, IonBuilder::inlineSimdCheck manually sets the Guard flag when creating an MSimdUnbox. However, EagerSimdUnbox.cpp and TypePolicy.cpp don't currently set the Guard flag for their SIMD unbox operations, even though they're still supposed to throw if their operand is not actually SIMD.

It seems that MSimdUnbox should set he Guard flag in its constructor as most other classes that use it do, since looking at the CodeGenerator code for LSimdUnbox, it appears that all LSimdUnbox instances are fallible.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

If anybody wants to take it, feel free, otherwise i'll do it when i'm back from pto.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

So the patch is easy, but finding a test case that triggers the issue without the patch isn't...

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: unbox-guard.patch

With a test case that triggers on inbound but not with this patch! (it was hard to trigger, as SIMD values are objects and thus bailouts could be produced by typebarriers or arguments checks)

Comment 4

[Dan Gohman [:sunfish]]

Comment on attachment 8592278 [details] [diff] [review]
unbox-guard.patch

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

Nice job finding a suitably obstreperous testcase :-).

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1fd18912545

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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