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

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 30 14:55:35 PST 2018


aheejin added a comment.

In D54873#1315295 <https://reviews.llvm.org/D54873#1315295>, @sbc100 wrote:

> In D54873#1315289 <https://reviews.llvm.org/D54873#1315289>, @aheejin wrote:
>
> > Ping. So the decision points are
> >
> > 1. Whether `SigIndex` should be in or out of `WasmEventType`?
> >   1. If `SigIndex` should be out of `WasmEventType`, should there even be `WasmEventType` anymore? Because then there's only `Attribute` left.
> > 2. It seems to be people don't like the term `Attribute`. Should we change it to `Kind` or something? We can change the spec text, because it's not set in stone or anything.
>
>
> Seems like we should keep the two member WasmEventType.  We need this for specifying the events themselves, and also for importing events.  It seems like this matches WasmGlobalType,


I'm not really sure what this means. Do you mean we should keep `SigIndex` as a part of `WasmEventType` and drop this CL? And why do we need this for specifying events themselves and for importing events? I don't think I understand what 'this' is actually. And why would it match `WasmGlobalType`? And do you also suggest, when we get to add multi-value globals later, we should insert `SigIndex` as a part of `WasmGlobalType` too?

> `Attrbiute` does seem like a strange name, but maybe there is reason that was chosen in spec currently?

I don't know. Karl chose it. I'm fine with the name, but I heard some complaints about it. If it was not you maybe it was @dschuff. Not that I want to change that myself, but I was just suggesting that it's possible to change the spec text also because it's not fixed yet.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54873





More information about the llvm-commits mailing list