925962 - Track expected contents of stack type sets in compiler constraints

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

Right now the CompilerConstraintList accumulated during IonBuilder tracks dependencies on heap type sets, but not stack type sets (which are all frozen at once after the IonBuilder finishes).  With bug 785905 stack type sets are free to mutate during compilation and when finishing it would be good to immediately invalidate the compilation if any of the stack type sets for the original or inlined scripts changed in the interim.

The attached patch does this by having the builder snapshot the expected contents of the stack type sets when it first runs, and checks the snapshot for any changes when finishing, invalidating the compilation if necessary.  The compiler is allowed to add new types to the snapshot if it wants to reflect any additional types it can tolerate being read or passed in at specific points.

Comment 1

[Jan de Mooij [:jandem]]

Comment on attachment 816116 [details] [diff] [review]
patch

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

::: js/src/jit/MIR.cpp
@@ +2924,4 @@
>  
>      types::TemporaryTypeSet *types = obj->resultTypeSet();
>      if (!types || types->unknownObject()) {
> +        observed->addType(types::Type::AnyObjectType(), alloc);

Shouldn't we check the return value of the addType calls in this function like the addType calls in IonBuilder?

Comment 2

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ea09c8bf385

Comment 3

[Brian Hackett [Laid off!]]

Followup to fix some jit-test failures:

https://hg.mozilla.org/integration/mozilla-inbound/rev/755ecb4d6e2c

Comment 4

[Phil Ringnalda (:philor)]

Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/27921f21cddf because the followup still had some shell failures (https://tbpl.mozilla.org/php/getParsedLog.php?id=29099418&tree=Mozilla-Inbound) and meantime lots more showed up, https://tbpl.mozilla.org/php/getParsedLog.php?id=29099706&tree=Mozilla-Inbound in nearly every debug test suite, perhaps the same thing or perhaps not as a near-silent SEGV in every ASan run, https://tbpl.mozilla.org/php/getParsedLog.php?id=29099377&tree=Mozilla-Inbound

Comment 5

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/81b505e9a435

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/81b505e9a435

Comment 7

[Alon Zakai (:azakai)]

Bullet workloads 1-4 seem to have regressed by 25-33% when this landed, none of the other 3 patches in that change list look suspicious. Could this have caused it?

http://arewefastyet.com/#machine=11&view=breakdown&suite=asmjs-apps