dblaikie added inline comments.
================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+ case Triple::Wasm:
+ return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+ ~0);
----------------
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.
================
Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:24-30
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU. Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
----------------
Given that S.plit DWARF type units don't require comdat support (the dwp tool
will be aware of the type units and parse their boundaries, read their type
hash from the TU Header, etc). /may/ be worth separating that
functionality/testing from the comdat support/testing - but maybe not all that
interesting to separate it into two separate patches. (& if you find the test
coverage works better in one test, that's OK - just a thought)
================
Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:47
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x00000000: Type Unit: {{.*}} version = 0x0004, abbr_offset
----------------
aardappel wrote:
> I guess this doesn't actually test that only one copy of these remain after
> linking? That's already tested in LLD?
Certainly lld shouldn't be tested here, no (llvm tests can't depend on other
subprojects like lld) - but yeah, some kind of comdat deduplication tests
should exist in lld (they don't have to be DWARF specific - the DWARF type
deduplication is meant to piggy-back on the existing comdat deduplication
infrastructure in thel inker)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88603/new/
https://reviews.llvm.org/D88603
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
