898486 - Don't extend lifetimes of dead vregs in interrupt check snapshots

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

The main place where snapshots appear in the LIR when running with --no-asmjs on asm.js code are in loop interrupt checks.  If the interrupt check triggers a GC we need to be able to resume execution in the interpreter/baseline.  Since we don't do a full liveness analysis to prune dead operands from snapshots, these snapshots can keep dead things live and hurt regalloc.  However, live range based regalloc *is* doing a liveness analysis, and we can leverage that liveness information to do more pruning.  The attached patch does this, when possible.  Pruning is only done from snapshots associated with interrupt checks, as other snapshots may represent a state predating their associated instruction and lead to an incorrect state after resuming in the interpreter/baseline.

Comment 1

[Brian Hackett [Laid off!]]

Forgot to add, this reduces the amount of spill code executed on asmjs-ubench-fannkuch by 10%.  The fraction of spill code executed is still about 4% more with --no-asmjs than with --asmjs for reasons I don't know yet.  Maybe just a quirk due to using different frontends, the executed LIR ops are more or less identical and there don't seem to be any snapshots entraining stuff on hot paths with --no-asmjs.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: updated

Fix a bug in the earlier patch: when an instruction is first visited during live range computation we don't necessarily know all live vregs at that point, as vregs may be carried back over loop backedges without associated phis.  This patch addresses this by delaying elimination of snapshot inputs until live intervals in loop bodies have been extended with such vregs.

Comment 3

[Jan de Mooij [:jandem]]

Sorry for taking a while to get to this. I'm a bit worried about the complexity this adds to code that's already complicated enough (regalloc). It also doesn't seem to help on the major benchmarks including Octane-mandreel. How much of a speedup do you see on fannkuch?

We've had a number of bugs caused by us not setting the Folded flag on phis, breaking phi elimination, bug 898047 being the most recent. Does this patch require us to also set the Folded flag on non-phis? For instance when DCE eliminates uses?

Comment 4

[Brian Hackett [Laid off!]]

We're already required to set the Folded flag on non-phis, as required by EliminateDeadResumePointOperands.  That function is a lighter weight but more general form of this optimization.

I don't see much of a speedup on fannkuch, the main reason I want to get this patch in is it allows us to (in theory) generate the same code (including spill code) with and without asm.js.  Until there is a measurable speedup from doing this though I'm ok with sitting on the patch.

The benchmarks are not terribly relevant to many of the optimizations that improve asm.js style code.  Even octane-mandreel isn't all that interesting, since it runs for such a short time and does so much compilation.  Another example is regalloc, the pit the backtracking allocator is stuck in is that it is generally a large improvement on asm.js style code but hurts octane and the other benchmarks.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 782285 [details] [diff] [review]
updated

Clearing flag for now per comment 4 :)