[PATCH] D88603: [WebAssembly] Add support for DWARF type units

Wed Oct 14 14:45:07 PDT 2020

dschuff added inline comments.

================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+    return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+                               ~0);
----------------
dblaikie wrote:
> dschuff wrote:
> > dblaikie wrote:
> > > dschuff wrote:
> > > > dschuff wrote:
> > > > > I may add a couple more tests to this, but I did want to ask @sbc100 about this, since I'm not 100% sure at the uniqueID field is for.
> > > > also let me be more clear about the question here: what is `UniqueID` for, and is it bad that I'm just passing it a number that is totally not unique?
> > > For ELF, at least, I believe the unique ID is used to know which elements are to be treated as part of the same deduplication set.
> > > 
> > > If Wasm support in lld does the same thing, then using the same number for every type unit would mean the linked binary would end up with only one type definition - even when the input has many varied/independent type definitions. Likely not what's intended.
> > For wasm I had thought that was what the 3rd argument (Group) was for. So if that's what `UniqueID` is for, then I have the same question about Group :)
> Oh, fair enough - I hadn't read closely. Yeah, guess it's up to you folks/how the wasm object format works... - so I'm with you on the "what is the uniqueID field for" (& what's the other field that's taking the hash?) & I'll leave it to you folks to... hash out.
@sbc100 ping, is this what we want here?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603