[PATCH] D54096: [WebAssembly] Add support for the event section

[Wouter van Oortmerssen via Phabricator via llvm-commits]

aardappel added inline comments.


================
Comment at: include/llvm/BinaryFormat/Wasm.h:72
+struct WasmEventType {
+  uint32_t Attribute;
+  uint32_t SigIndex;
----------------
Some comments as to what these refer to, esp the uint32_t types?


================
Comment at: include/llvm/MC/MCSymbolWasm.h:27
+  wasm::WasmEventType EventType;
+  bool EventTypeSet = false;
 
----------------
nicer to have an "invalid" member of `WasmEventType` ?


================
Comment at: lib/CodeGen/AsmPrinter/WasmException.cpp:22
+void WasmException::endModule() {
+  // This is the label used in 'throw' instruction to denote this is a C++
+  // exception. This label has to be emitted somewhere once in the module. Check
----------------
why do we use a label for this?


================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp:121
+  for (auto Ty : Sym->getSignature()->Params)
+    OS << ", " << WebAssembly::TypeToString(Ty);
+  OS << '\n';
----------------
We should maybe decide to make all signatures look the same, so they can parsed equally (both by the assembler and a human :)
We should merge .param and .return? Should that have a symbol name much like .eventtype? Use void instead of just omitting .return?


Repository:
  rL LLVM

https://reviews.llvm.org/D54096