Closed
Bug 925962
Opened 11 years ago
Closed 11 years ago
Track expected contents of stack type sets in compiler constraints
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter 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 | ||
Updated•11 years ago
|
Assignee: nobody → bhackett1024
Comment 1•11 years ago
|
||
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+
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Followup to fix some jit-test failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/755ecb4d6e2c
Comment 4•11 years ago
|
||
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
Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment 7•11 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•