[llvm] [DebugInfo] Preserve line and column number when merging debug info. (PR #129960)

[Snehasish Kumar via llvm-commits]

snehasish wrote:

> > Debug info loss on merged indirect calls is particularly important since it disallows indirect call promotion.
>
> Could you walk me through this in more detail - exactly what DWARF is generated, how does it get consumed, and what decisions are made based on it?

[Here](https://reviews.llvm.org/D148876#4289326) is a pointer to a prior discussion on merging value profile metadata which notes the need to retain debug information to be able to assign the right profile information to indirect calls. For AFDO we rely on the start line of the function (needs `-fdebug-info-for-profiling`) and the line number of the indirect call (`-g1` is sufficient) to compute the line offset. This is used to generate a profile like this [example](https://github.com/llvm/llvm-project/blob/94426df66a8d7c2321f9e197e5ef9636b0d5ce70/llvm/test/Transforms/SampleProfile/Inputs/fnptr.prof#L9). Here Line 9 in the profile has 2 entries which indicate the targets for the indirect call and their counts. In the absence of line numbers for a merged indirect call we fail to map the profiled targets to the indirect call in the IR. Note that retaining line number information is the first step to enabling more effective ICP (details in D148876). So the 0.2% number I shared is conservative and there is more room for improvement!

> > I think this is probably the most important thing: documenting the principles of what we do, why, what AFDO needs, what profilers need, etc, what modes we have, and what we expect them to do.
> 
> Documenting it seems good - perhaps more than has been posted so far (the AFDO documentation doesn't discuss the issue of "samples of a source location inside a block could lead to misconstrued assumptions about that block being reachable"). But even then, I'm still really worried that this makes the needs of different consumers divergent to a problematic degree - we managed to make the direction align between a variety of consumers (humans, profilers, profile driven optimization, etc) so far and diverging from that seems like a real problem. (even just at Google - humans use profiles from this same debug info, and if we tell them that some block is hot when it isn't, that seems potentially really unfortunate)

I agree - with this change we are no longer have a single aligned implementation for all users. However, the cost we pay in terms of performance for alignment increases with time as more aggressive optimizations are introduced. For newer optimizations such as MemProf the loss of debug information on call insts is a significant contributor to coverage loss.  Basically, the trade off we have encoded in LLVM increases the downside for PGO with the passage of time. 

With this change I want us to be able to quantify the impact in production. It's possible we will encounter situations where this is not ideal (e.g. crash stack traces) but that's ok in my opinion. In the short term we can disable the change for those affected binaries and in the long term, gives us motivation to champion work on improving source location fidelity for humans and tools. 

Are there any further changes / clarifications I can make to help move this along? @rnk @dwblaikie 


https://github.com/llvm/llvm-project/pull/129960