In D100043#3188388, @lzaoral wrote:

Thanks a lot @delcypher! Can you (or anyone else with commit rights) commit it to trunk for me?

LGTM

LGTM

In D115325#3224922, @Orlando wrote:

Looking at the DWARFv5 spec while trying to get my head round pubnames, I see that there's a v5 specific section that combines pubnames and pubtypes called debug_names. Sorry if this is a silly question - this part of DWARF is new to me - does clang support emitting debug_names?

LGTM

Code looks quite reasonable; the test needs a little cleanup.

Presumably we can generate the team memberships from the Bugzilla component auto-subscribers easily enough, even if it's manual.
I take it we'd need one of these sub-teams per label (mostly); is there a max number of teams?

Ping

Address review comments.

Fix warning capitalization

Tidy up error handling for more consistency.
Ignore unsupported symbols.

Remove more unnecessary sections/fields from yaml file.
Report filename in a warning message.
Unconditionally demangle function name for future-proofing.

Before:

OVERVIEW: Create an object file from a YAML description
USAGE: yaml2obj [options] <input file>

In D111358#3144735, @MaskRay wrote:

What this tool does fit a bit into the realm of llvm-ifs (it had a previous name "llvm-elfabi" D100139. I like "elfabi" better as "ifs" seems confusing). Would it be suitable to move the code there?

LGTM

In D113633#3127799, @ljmf00 wrote:

Also, take the example of https://reviews.llvm.org/D113634 , which may be split into two patches for further discussion. Anyway, I tend to agree with you, I can join everything together in D113634.

Typically I've done "support" for a tag or other DWARF feature in several phases, each usually (not necessarily) with its own patch. Each phase builds on the previous one, which is the project's preferred incremental-development model.

This should be in the same patch as the code changes, D113633.

In D113572#3122660, @ljmf00 wrote:

@probinson Can you check now? If it looks good, can you land it, please?

In general doing a clang-format before extensive work on some files is an obvious NFC, but I can see why you might have wanted some review on this.

LGTM

I'd rather see this as part of Verbose. The DIEs are indented to show nesting already, so the explicit parent offset is really additional information.

The lines where CHECK is split into CHECK-LINUX and CHECK-NOLINUX are also enabling the test for nvptx.

Sorry it took a while to cycle back to this.

Updated the UI:
Removed --verbose, replaced with --report=[summary,discrepancy,full]
Added --dump-tli to get info about TLI without reference to a library file

Sorry for the long delay, between CPPcon and putting together a dev-meeting talk I've been pretty distracted.

Given that enum is really the only problematic case, LGTM.

I'm not a verifier expert, but I'm also not convinced the verifier change is doing quite the right thing. Yes, it finds the case we ran into, but it seems like it's just checking for that one bogus case where the containing scope is incorrectly an enum. It seems to me that proper scope-checking should be looking for the tag to be something that we agree should be allowed to be a scope, rather than looking for the tag to be one thing (of many?) that we agree should not be a scope.

+ @jhenderson for object-file-reading expertise.

+folks who participated in D107509

I suspect this is not the only way for directories and filenames to become confused, but it certainly helps. LGTM.

Thanks for the ping, I hadn't added this to my to-do list. LGTM

One minor thing and LGTM.

Seems like a good simplification. LGTM.

Is there a config tweak needed to get this to run properly for us? Given the upstream Windows bots are not seeing a problem.

We were seeing this in a downstream windows bot, although it doesn't seem to affect any public bots. If anyone has suggestions about why that might be, we're open to fiddling our downstream bot.

It may feel like I'm being a bit picky, but the prologue_end business is a bit picky, and it's helpful to have the tests be as clear as possible about what's expected.

A bit of yak shaving because I had wanted to add __sqrt*_finite to annotate.ll, but it wasn't doing quite the right thing.

Split test into 3 parts.

As this patch has had to be reverted due to a completely baffling failure on one bot, we can reopen the debate about how to solve the original problem, which is: XFAIL and UNSUPPORTED allow triples (with substring handling), but REQUIRES doesn't know about triples at all.

In D110129#3013946, @yonghong-song wrote:

• The only thing left is for llvm/test/DebugInfo/X86/attr-btf_tag-typedef.ll for which I didn't use %itanium_abi_triple as it seems only available for %clang ... and not available for llc.

I've poked around in the copy of gdb that we're using for testing; I see it noting DW_AT_defaulted, and factoring it into pass by reference/value decisions; but, if DW_AT_calling_convention is present, the latter takes precedence.
It looks like CGDebugInfo::CreateLimitedType will always set the CC flags for a CXXRecordDecl, and I think we always go through that path for complete type descriptions, so I accept that DW_AT_defaulted isn't really necessary.

In D109970#3006806, @dblaikie wrote:

Any particular use case in mind for this debug info? (generally I'd rather we not emit extra DWARF we don't have some use case for in mind)

I think this is the last of the new DWARF v5 tags/attributes.

In D106621#2973954, @dblaikie wrote:

Looks generally OK to me. (not 100% sure that DW_TAG extension space is unused, but I think we've generally got most of the extensions in llvm-dwarfdump, so it probably is unused - and worst-case we can change it if needed)