This is an archive of the discontinued LLVM Phabricator instance.

WebAssembly: emit `(func (param t) (result t))` s-expressions
ClosedPublic

Authored by jfb on Aug 24 2015, 6:03 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jfb updated this revision to Diff 33033.Aug 24 2015, 6:03 PM
jfb retitled this revision from to WebAssembly: emit `(func (param t) (result t))` s-expressions.
jfb updated this object.
jfb added a reviewer: sunfish.
jfb added a subscriber: llvm-commits.
jfb added a comment.Aug 24 2015, 6:06 PM

This is missing a few test updates for param and result, will do them if the overall changes looks good.

jfb updated this revision to Diff 33106.Aug 25 2015, 1:06 PM
  • Update tests to have param / result.
jfb updated this revision to Diff 33135.Aug 25 2015, 3:27 PM
  • Update .globl test for output (func
sunfishcode accepted this revision.Aug 25 2015, 3:39 PM
sunfishcode added a reviewer: sunfishcode.
sunfishcode added a subscriber: sunfishcode.
sunfishcode added inline comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
106 ↗(On Diff #33135)

Please put an assert or llvm_unreachable or something here to make sure this gets caught.

145 ↗(On Diff #33135)

This is subjective, but my preference would be to omit this comment, as it adds clutter. If you want to keep it, would you mind making it lower-case?

This revision is now accepted and ready to land.Aug 25 2015, 3:39 PM
jfb updated this revision to Diff 33143.Aug 25 2015, 3:55 PM
jfb marked an inline comment as done.
jfb edited edge metadata.
  • Lowercase 'end func'.
  • Add llvm_unreachable.
jfb marked an inline comment as done.Aug 25 2015, 3:56 PM
jfb added inline comments.
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
145 ↗(On Diff #33135)

Lowercased.

I'd like to keep it since functions won't be too frequent compared to the expression trees. At least for early testing it'll make things easier to grok IMO.

This revision was automatically updated to reflect the committed changes.
jfb marked an inline comment as done.