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: > 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. ================ 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.) ---------------- dschuff wrote: > dblaikie wrote: > > 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) > I copied this test more-or-less directly from X86 :) > Although I will say it's kind of nice to test all the header types in one go > like this just because it's slightly annoying to construct the LLVM metadata > for debuginfo tests, so it's nice to be able to share that as much as > possible. Previously we just didn't have enough coverage for split-dwarf > anyway. Ah, fair enough - symmetry sounds good :) 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
