[PATCH] D109970: [DebugInfo] Support DW_AT_defaulted

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D109970#3007280 <https://reviews.llvm.org/D109970#3007280>, @probinson wrote:

> In D109970#3006806 <https://reviews.llvm.org/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 don't think Sony has a use for it.  The description in section 5.7.8 suggests in/out of class defaulting can affect calling conventions,

I think that came out of a misunderstanding that ultimately resulted in a more robust/complete solution involving the `DW_AT_calling_convention` attribute.

> and I see gcc emitting it, so I figured there was likely some use for it.  I suppose I could see if any gdb tests would start working because of this, but apart from that, no I don't have a use-case in mind.

Yeah, if there's any evidence this is being used, that'd be helpful I think - otherwise I'd be inclined to leave it until there's a motivating use case (especially given, to the best of my understanding of the conversations at the time, where this came out of & that `DW_AT_calling_convention` ended up being necessary anyway)

>> (& if/when this is committed, should be in 3 parts - dwarfdump support, IR/codegen support (can be split into two if you like, but doesn't have to be), and Clang support)
>
> I did originally have this as separate LLVM and Clang commits, happy to split it back up.

I don't mind/think it's good to review it together, but separable parts should be committed separately when it comes to that.

>   The dwarfdump support already existed, there just wasn't a test, so I tucked that into the IR/codegen changes; I guess adding the dump test could be its own thing. 

Yeah, that'd be great if you could commit the test coverage separately/ahead of the rest of any of this.

> CodeGen is what makes use of the IR flags, splitting them means one patch defines new IR flags that... aren't used for anything?  I figure they're better off in one patch.

Yeah, that's a marginal area where splitting may or may not be done - we're not especially serious about that. Though it can help separate a lot of mechanical work to plumb the stuff through IR (& that work will have its own test coverage - IR parsing/writing/bitcode/etc roundtripping) - even if it has no observable effect on the whole invocation of clang/llc/etc. Separating all that mechanical stuff from a few lines of semantic/more nuanced code in lib/CodeGen/AsmPrinter and testing those pieces separately.


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

https://reviews.llvm.org/D109970