[PATCH] D54873: [WebAssembly] Make signature index not a part of EventType

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: include/llvm/BinaryFormat/Wasm.h:92
 struct WasmEvent {
   uint32_t Index;
   WasmEventType Type;
----------------
sbc100 wrote:
> I also think it nice if you can get the signature from the WasmEvent stuct.    Rather than separating the events from their signatures.  Seems to fit better with the binary representation.
I don't know that I have an opinion about whether we have a duplication like in InputGlobal, but I do think that however we handle signatures for functions should be the same as what we do for events. Globals are different because they have a wasm value type instead of a signature, but @aheejin pointed out that with the multi-value proposal they will also have a signature in the same index space. So the way we handle the signatures should be uniform (which I guess is the point of this CL...)


I think part of the confusion here is that this file has some code that is intended to directly model the binary format (which is why `WasmEventType` had the `SigIndex` field in the first place) and some that's intended to be useful at a slightly higher level for MC and the linker. e.g. why we have `WasmSignature*` in some places and signature indices in others.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54873