[PATCH] D111086: [WebAssembly] Remove WasmTagType
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 4 16:11:01 PDT 2021
sbc100 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:
> > 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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111086/new/
https://reviews.llvm.org/D111086
More information about the llvm-commits
mailing list