This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Print an extra local decl when the user stack pointer is used
ClosedPublic

Authored by dschuff on Dec 15 2015, 4:20 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

dschuff updated this revision to Diff 42935.Dec 15 2015, 4:20 PM
dschuff retitled this revision from to [WebAssembly] Print an extra local decl when the user stack pointer is used.
dschuff added reviewers: sunfish, jfb.
dschuff added a subscriber: llvm-commits.
dschuff added a subscriber: azakai.Dec 15 2015, 4:21 PM
jfb edited edge metadata.Dec 15 2015, 4:23 PM

lgtm after updating the test.

test/CodeGen/WebAssembly/userstack.ll
10 ↗(On Diff #42935)

Terminate with {{$}} here and below.

sunfish added inline comments.Dec 15 2015, 4:28 PM
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
185 ↗(On Diff #42935)

This works, but it's a little odd that the WebAssemblyFunctionInfo has PhysRegs as a general-purpose thing, but we have a special case for SP here. Would be be feasible to iterate over PhysRegs here instead?

test/CodeGen/WebAssembly/userstack.ll
10 ↗(On Diff #42935)

These CHECK lines would be good candidates for {{$}} at the end, to ensure that we have exactly the right number of locals.

dschuff updated this revision to Diff 42939.Dec 15 2015, 4:28 PM
dschuff edited edge metadata.
  • Update tests for review
dschuff marked 2 inline comments as done.Dec 15 2015, 4:34 PM
dschuff added inline comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
185 ↗(On Diff #42939)

well, IIUC it would be "iterate over phys regs except for ARGUMENTS and EXPR_STACK" which would leave us with SP and FP. so approximately the same level of weirdness either way?

sunfish added inline comments.Dec 15 2015, 4:58 PM
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
185 ↗(On Diff #42939)

I mean iterate over MFI->PhysRegs and only look at elements that are not -1U. That way if we implement MFI->adjustsStack() or hasFP(MF) or whatever in WebAssemblyFrameInfo.cpp, we won't need to update this code too.

I think you can extend WebAssemblyAsmPrinter::getRegType to know about physregs using TRI->getMinimalPhysRegClass.

dschuff updated this revision to Diff 42946.Dec 15 2015, 5:24 PM
  • Iterate over phys regs instead
dschuff marked an inline comment as done.Dec 15 2015, 5:25 PM
dschuff updated this revision to Diff 42947.Dec 15 2015, 5:26 PM
  • Remove now-unused include
sunfish accepted this revision.Dec 15 2015, 7:04 PM
sunfish edited edge metadata.

Looks good, thanks!

This revision is now accepted and ready to land.Dec 15 2015, 7:04 PM
This revision was automatically updated to reflect the committed changes.