[PATCH] D111086: [WebAssembly] Remove WasmTagType

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lld/wasm/InputFiles.cpp:499
   ArrayRef<uint32_t> funcTypes = wasmObj->functionTypes();
+  ArrayRef<uint32_t> tagTypes = wasmObj->tagTypes();
   ArrayRef<WasmSignature> types = wasmObj->types();
----------------
aheejin wrote:
> sbc100 wrote:
> > aheejin wrote:
> > > sbc100 wrote:
> > > > Why have a separate  `tagTypes()` method?  Wouldn't be better just add `Type` o `Sig` to `struct WasmTag`? 
> > > It's not an inherently property of `Tag` and will become invalidate at linking time. This is the same as how we handle function types now; see one line above.
> > Just because its going to be re-written/updated by the linker doesn't mean the libObject interface should not expose it directly as part of the Tag.
> > 
> > I'm trying to remember why `functionTypes` is not part simply embedded the `WasmFunction` array.    Perhaps because there are function types for imported functions as well as defined ones?     Actually that is not true since imported functions *do* embed thier signature:
> > 
> > ```
> > struct WasmImport {                                                                                               
> >   StringRef Module;                                                              
> >   StringRef Field;                                                               
> >   uint8_t Kind;                                                                  
> >   union {                                                                        
> >     uint32_t SigIndex;                 <--- here                                             
> >     WasmGlobalType Global;                                                       
> >     WasmTableType Table;                                                         
> >     WasmLimits Memory;                                                           
> >     WasmTagType Tag;                                                             
> >   };                                                                             
> > };    
> > ```
> > 
> > Unless there is some other reason to keep tagTypes separate I suggest you simply embed it them.
> I wondered about that too. Also there are two separate `WasmFunction`s, to make matters more confusing.. Anyway, I wanted to treat functions and tags consistently. How about doing it for both functions and tags as a followup?
Oh, you already did that in D111104, thanks. I'll rebase this onto it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111086