Closed Bug 867070 Opened 7 years ago Closed 7 years ago
Monkey: v8-raytrace is faster when disabling LICM
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.
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)
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 #744696 - Flags: review?(dvander) → review+
This was backed out while investigating Windows PGO bustage. Re-landed. https://hg.mozilla.org/integration/mozilla-inbound/rev/6b1f607fd243
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.