This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Emit a BasePointer when we have overly-aligned stack objects
ClosedPublic

Authored by jgravelle-google on Nov 2 2016, 2:37 PM.

Details

Summary

Because we shift the stack pointer by an unknown amount, we need an
additional pointer. In the case where we have variable-size objects
as well, we can't reuse the frame pointer, thus three pointers.

Event Timeline

jgravelle-google retitled this revision from to [WebAssembly] Emit a BasePointer when we have overly-aligned stack objects.
jgravelle-google updated this object.
jgravelle-google added reviewers: dschuff, sunfish.
jgravelle-google added a subscriber: llvm-commits.
  • Move HasBP closer to its use
dschuff added inline comments.Nov 2 2016, 5:45 PM
lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
40

Maybe clarify that "overaligned" means more aligned than the stack pointer (rather than more than natural alignment for the slot itself).

test/CodeGen/WebAssembly/alignment.ll
1

alignment.ll seems a bit generic. It could probably be stack-aligment or else these tests could just be added to userstack.ll

29

Might as well check for the value of the const too, so we know the alignment is 16.

34

Seems like we should be storing directly from BP. Where does this copy_local come from?
The epilog codegen looks ok; I guess this copy is the one generated in the prolog and gets moved down by the stackifier?

test/CodeGen/WebAssembly/alignment.ll
34

Huh. This is caused by us emitting a frame pointer, and register stackification doing magic to it.
tl;dr: Yes.

Prolog/epilogue:

  1. load __stack_pointer into SP
  2. copy SP into BP
  3. decrement SP by fixed amount
  4. store new SP as FP
  5. [rest of code]
  6. store BP into __stack_pointer

Reg stackify:

  1. Moves the single copy into BP to be next to its use
  2. Tees the two uses of the original load of __stack_pointer, stackifies one for use in computing FP, and saves one in BP

The good news is this just gets translated to get_local in s2wasm. And adding -wasm-explicit-locals models it that was as well, so there's no actual cost here, it's just kinda quirky.
... is what I've been able to piece together.

  • Simplify SPReg, clarify comment, rename test, check stack size in test
dschuff accepted this revision.Nov 7 2016, 11:39 AM
dschuff edited edge metadata.
This revision is now accepted and ready to land.Nov 7 2016, 11:39 AM
This revision was automatically updated to reflect the committed changes.