Closed Bug 925962 Opened 7 years ago Closed 7 years ago

Track expected contents of stack type sets in compiler constraints

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #816116 - Flags: review?(jdemooij)
Assignee: nobody → bhackett1024
Blocks: 925194
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?
Attachment #816116 - Flags: review?(jdemooij) → review+
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
Depends on: 927544
https://hg.mozilla.org/mozilla-central/rev/81b505e9a435
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
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
Duplicate of this bug: 925194
Depends on: 951497
You need to log in before you can comment on or make changes to this bug.