Closed
Bug 867070
Opened 12 years ago
Closed 12 years ago
IonMonkey: v8-raytrace is faster when disabling LICM
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: h4writer, Assigned: h4writer)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
There is a 3% improvement when disabling LICM. Not sure how this happens, but will probably related to bailouts. This also happens for octane-mandreel (~8%), misc-bugs-608733-interpreter (~12%), misc-bugs-663087-decompress-gzip (~4.2%) ...
Interesting though is that ss-raytrace doesn't see this improvement and a debug build doesn't show the difference either.
Assignee | ||
Comment 1•12 years ago
|
||
This was most visible on misc-bugs-608733-interpreter.js.
For some instructions we don't want to hoist them, if it doesn't enable us to hoist more. Most apparent is MConstant. In most cases we are using it at its uses. Hoisting that instruction increases register pressure, since it is live longer.
Currently I hardcoded MConstant/MConstantElements and MBox. Do you think I need to add a function to MDefinition that defaults to "always hoist", but can be overriden to "Only hoist when one of its uses is loop invariant" ?
This patch fixes the difference between --ion-licm=on/off on misc-bugs-608733-interpreter.js. It looks like it also fixes v8-raytrace.js totally, but since it jumps, I can't guarantee. It could still be slightly slower. But I think we got a ~3% improvement on it.
Assignee: general → hv1989
Attachment #743528 -
Flags: feedback?(dvander)
Assignee | ||
Comment 2•12 years ago
|
||
The feedback that this was indeed a good approach was given during skype session.
The extra removed code is actually really stupid. We should never hit this, since all hoistable instructions are added to the worklist and we see definitions before uses. I.e. operands are seen before the instruction. I verified and this extra code doesn't add extra hoistable instructions. As a side node there was a fault in that idea. Since during popFromWorklist, we removed it from the list. Resulting in that instruction getting added again. Needing more memory ...
=> So the removal of that code doesn't change the resulting MIR/LIR at all, but decreases memory and speed of traversal. (Normally shouldn't be noticeable on normal workloads)
Attachment #743528 -
Attachment is obsolete: true
Attachment #743528 -
Flags: feedback?(dvander)
Attachment #744696 -
Flags: review?(dvander)
Updated•12 years ago
|
Attachment #744696 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 3•12 years ago
|
||
Comment 4•12 years ago
|
||
This was backed out while investigating Windows PGO bustage. Re-landed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b1f607fd243
Comment 5•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•