This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add '.eventtype' directive support
ClosedPublic

Authored by aheejin on Dec 5 2018, 9:51 PM.

Details

Summary

This patch supports .eventtype directive printing and parsing in the
same syntax with .functype.

Diff Detail

Event Timeline

aheejin created this revision.Dec 5 2018, 9:51 PM
aheejin updated this revision to Diff 176928.Dec 5 2018, 10:16 PM
  • clang-format & clang-tidy
aheejin marked an inline comment as done.Dec 5 2018, 10:53 PM
aheejin added inline comments.
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
463

I don't really know what this TODO means, but because other directives have it, I just pasted it again here.

sbc100 added a comment.Dec 6 2018, 8:38 AM

Since event types can't have return values (they are always no_return right?) should we split out "parseParamList" and have them look like this?

.eventtype  __cpp_exception i32
.eventtype  some_other_exception i32, i32
sbc100 accepted this revision.Dec 6 2018, 8:39 AM

lgtm % comment

This revision is now accepted and ready to land.Dec 6 2018, 8:39 AM
aardappel accepted this revision.Dec 6 2018, 10:55 AM
aardappel added inline comments.
lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
463

This comment is related to the comment above about WebAssemblyAsmPrinter::EmitFunctionBodyStart. Effectively it means that compared to that code, we're missing this particular emit, but it doesn't seem to matter

aheejin updated this revision to Diff 177093.Dec 6 2018, 6:22 PM
  • Print only param list for eventtype

Since event types can't have return values (they are always no_return right?) should we split out "parseParamList" and have them look like this?

.eventtype  __cpp_exception i32
.eventtype  some_other_exception i32, i32

Done.

aheejin updated this revision to Diff 177095.Dec 6 2018, 6:24 PM
  • Minor fix
sbc100 accepted this revision.Dec 7 2018, 10:56 AM
This revision was automatically updated to reflect the committed changes.