Closed
Bug 1072911
Opened 7 years ago
Closed 7 years ago
Scalar Replacement: Add MIRType to MPhi & M*State definitions
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: nbp, Assigned: nbp)
References
Details
Attachments
(1 file)
3.63 KB,
patch
|
h4writer
:
review+
|
Details | Diff | Splinter Review |
See tests cases from bug 1069307 comment 1.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8495235 -
Flags: review?(hv1989)
Assignee | ||
Comment 2•7 years ago
|
||
Comment on attachment 8495235 [details] [diff] [review] Scalar Replacement: Add missing MIRTypes. Review of attachment 8495235 [details] [diff] [review]: ----------------------------------------------------------------- Note: I added the last 2 test cases in the patch as the first test case do an infinite loop and highlight the same assertion.
Comment 3•7 years ago
|
||
Comment on attachment 8495235 [details] [diff] [review] Scalar Replacement: Add missing MIRTypes. Review of attachment 8495235 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/ScalarReplacement.cpp @@ +310,5 @@ > // nodes. > succState = BlockState::Copy(alloc_, state_); > size_t numPreds = succ->numPredecessors(); > for (size_t slot = 0; slot < state_->numSlots(); slot++) { > + MPhi *phi = MPhi::New(alloc_, MIRType_Value); No need for this change. MPhi has a default resultType of MIRType_Value @@ +764,5 @@ > // nodes. > succState = BlockState::Copy(alloc_, state_); > size_t numPreds = succ->numPredecessors(); > for (size_t index = 0; index < state_->numElements(); index++) { > + MPhi *phi = MPhi::New(alloc_, MIRType_Value); No need for this change. MPhi has a default resultType of MIRType_Value
Attachment #8495235 -
Flags: review?(hv1989) → review+
Assignee | ||
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/11a4a935eb7d
Comment 5•7 years ago
|
||
there appears to be a regression (19672 -> 17651) on Octane-Splay on machine 11 on AWFY. The regression range is this from AWFY : http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=846faaa68219&tochange=808aa539aad1
Updated•7 years ago
|
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 6•7 years ago
|
||
Hannes, I do not think this can cause any performance impact as the only modifications are on unused objects/arrays. This issue is caused by the fix in Bug 1007213, and would be resolved by the optimization from Bug 1072188.
Flags: needinfo?(nicolas.b.pierron)
Comment 7•7 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #6) > Hannes, I do not think this can cause any performance impact as the only > modifications are on unused objects/arrays. This issue is caused by the fix > in Bug 1007213, and would be resolved by the optimization from Bug 1072188. Yeah that sound likely. Thanks. I'll keep an eye on bug 1072188. Let's hope it can land soonish so this regression is gone.
Comment 8•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/11a4a935eb7d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•