Page MenuHomePhabricator

[WebAssembly] Do register stackification and coloring late.
ClosedPublic

Authored by sunfish on May 9 2016, 11:07 AM.

Details

Summary

LiveIntervals was recently refactored to eliminate its dependency on LiveVariables, so it's now theoretically possible to run it in a late pass (with care). The patch attached here does this, so that it can also move RegStackify and RegColoring late also. That means that all code expanded by PEI, tail duplication, and other things is all automatically exposed to stackification and coloring.

This is an alternative to http://reviews.llvm.org/D19345 . Instead of moving PEI earlier, this pass moves stackification and coloring later.

Diff Detail

Repository
rL LLVM

Event Timeline

sunfish updated this revision to Diff 56600.May 9 2016, 11:07 AM
sunfish retitled this revision from to [WebAssembly] Do register stackification and coloring late..
sunfish updated this object.
sunfish added a reviewer: dschuff.
sunfish set the repository for this revision to rL LLVM.
sunfish updated this revision to Diff 56631.May 9 2016, 2:19 PM
sunfish updated this object.

Minor bugs fixed, and all the tests are now updated.

dschuff edited edge metadata.May 9 2016, 3:52 PM

Looks pretty good overall.

lib/Target/WebAssembly/WebAssemblyOptimizeLiveIntervals.cpp
73 ↗(On Diff #56631)

Wouldn't it make more sense to do MRI->leaveSSA() since we don't actually care whether it's SSA now? (likewise with the other new passes in this CL)

lib/Target/WebAssembly/WebAssemblyPrepareForLiveIntervals.cpp
16 ↗(On Diff #56631)

Can we add one statement saying briefly what LiveIntervals' requirements actually are?

lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp
87 ↗(On Diff #56600)

Does getUniqueVRegDef depend on SSA form? I guess this means WasmPEI now depends on SSA?

sunfish updated this revision to Diff 56652.May 9 2016, 5:33 PM
sunfish edited edge metadata.

Update for review comments. Fix a frame-index optimization bug when the constant has multiple uses.

sunfish updated this revision to Diff 56654.May 9 2016, 5:38 PM
sunfish marked an inline comment as done.

Address *all* the review comments this time.

sunfish marked 2 inline comments as done.May 9 2016, 5:40 PM
sunfish added inline comments.
lib/Target/WebAssembly/WebAssemblyOptimizeLiveIntervals.cpp
74 ↗(On Diff #56654)

Makes sense. Done.

lib/Target/WebAssembly/WebAssemblyPrepareForLiveIntervals.cpp
17 ↗(On Diff #56654)

Done. The comments in the code explain in more detail, but I added a brief description here.

lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp
88 ↗(On Diff #56654)

getUniqueVRegDef returns nullptr if the register has multiple defs, so this code is just being opportunistic right now. I added a comment about that.

dschuff accepted this revision.May 9 2016, 5:47 PM
dschuff edited edge metadata.

LGTM, 1 nit.

lib/Target/WebAssembly/WebAssemblyStoreResults.cpp
192 ↗(On Diff #56654)

Also leaveSSA() here

This revision is now accepted and ready to land.May 9 2016, 5:47 PM
This revision was automatically updated to reflect the committed changes.