This is an archive of the discontinued LLVM Phabricator instance.

WebAssembly: textual emission uses expected opcode names
ClosedPublic

Authored by jfb on Aug 5 2015, 1:36 PM.

Details

Summary

WebAssembly's tablegen instructions have the names WebAssembly expects, but by LLVM convention they're uppercase and suffixed with their type after an underscore. Leave the C++ code that way, but print outt he names WebAssembly expects (lowercase, no type). We could teach tablegen to do this later, maybe by using !cast<string>(node) in the .td files.

Diff Detail

Repository
rL LLVM

Event Timeline

jfb updated this revision to Diff 31390.Aug 5 2015, 1:36 PM
jfb retitled this revision from to WebAssembly: textual emission uses expected opcode names.
jfb updated this object.
jfb added a reviewer: sunfish.
jfb added a subscriber: llvm-commits.
sunfish accepted this revision.Aug 6 2015, 1:31 PM
sunfish edited edge metadata.
sunfish added inline comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
85 ↗(On Diff #31390)

Please avoid std::tolower since it has locale-specific behavior, and compiler output should not be locale-specific.

111 ↗(On Diff #31390)

We should really just teach the generic operand printing code below how to handle immediate operands, and then even the ARGUMENT opcodes can just go through the generic path.

This revision is now accepted and ready to land.Aug 6 2015, 1:31 PM
majnemer added inline comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
85 ↗(On Diff #31390)

FYI we have such a function in LLVM, ascii_tolower, but it would need to move to a header file.

jfb updated this revision to Diff 31500.Aug 6 2015, 6:56 PM
jfb marked an inline comment as done.
jfb edited edge metadata.
  • Address review comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
85 ↗(On Diff #31390)

I used StringRef::lower.

111 ↗(On Diff #31390)

Yes, will do in a follow-up.

This revision was automatically updated to reflect the committed changes.