This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Fix assembly printing of br_table
ClosedPublic

Authored by aheejin on Oct 22 2018, 11:24 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

aheejin created this revision.Oct 22 2018, 11:24 AM
aardappel added inline comments.Oct 22 2018, 2:27 PM
test/CodeGen/WebAssembly/stack-insts.ll
9 ↗(On Diff #170459)

If this is the only thing this file is testing, is "stack-inst.ll" not a bit generic sounding for what it actually does? Either we intend to add more stack instruction format testing to this file, or it should have a more specific name?

aheejin added inline comments.Oct 22 2018, 2:40 PM
test/CodeGen/WebAssembly/stack-insts.ll
9 ↗(On Diff #170459)

First I tried to create a .s test to test with llvm-mc so we can only test the parsing and printing, but it looks like [[ https://bugs.llvm.org/show_bug.cgi?id=39384 | br_table is not supported in WebAssemblyAsmParser ]] now. So I made this file, because all other .ll tests in this directory use non-stack version of instructions. The reason I named this file name stack-insts.ll was I thought maybe there will be cases we want to test misc. things for stack version of instructions, in which case we can add them to this file. But if you think for now this file should have more specific name, I think that also makes sense too.

aardappel accepted this revision.Oct 22 2018, 2:49 PM
aardappel added inline comments.
test/CodeGen/WebAssembly/stack-insts.ll
9 ↗(On Diff #170459)

No, just an observation, it's fine if we want to add more tests.
Btw, hadn't seen that bug, feel free to assign them to me in the future.

This revision is now accepted and ready to land.Oct 22 2018, 2:49 PM
This revision was automatically updated to reflect the committed changes.