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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin marked an inline comment as done.
aheejin added inline comments.


================
Comment at: include/llvm/BinaryFormat/Wasm.h:80
+  // Kind of event. Currently only WASM_EVENT_ATTRIBUTE_EXCEPTION is possible.
+  uint32_t Attribute;
+  uint32_t SigIndex;
----------------
sbc100 wrote:
> Should this be called Kind or Type?  Or is it more of a bitfield?
> 
> If its Type then perhaps that SigIndex should be in union:
> 
> e.g.
> 
> ```
> struct WasmEventType {
>   uint32_t Kind
>   union {
>     ExceptionSigIndex;
>   };
> };
> ```
> 
> still lgtm in any case as this stuff can be iterated on.
Oh I missed this comment when landing this. For `Attribute`, I tried to follow [the spec](https://github.com/WebAssembly/exception-handling/blob/master/proposals/Exceptions.md#other-types). If we change this to a bitfield, would the memory savings be big enough to be meaningful?

And why should `SigIndex` be within a union? By the way, I think `SigIndex` is better be outside of `WasmEventType` anyway, so I opened D54873.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54096/new/

https://reviews.llvm.org/D54096