[PATCH] D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics

Mon Feb 25 17:25:07 PST 2019

probinson added a comment.

In D58453#1409691 <https://reviews.llvm.org/D58453#1409691>, @jmorse wrote:

> Absolutely true -- and constant-valued dbg.values (including undef, technically) are frequently the source of bug reports (ex: PR40795) as their lifetime isn't associated with anything else in the IR (see also PR40628).

That's disheartening. Reading the PR it's apparent that the location list is failing to account for the dataflow computations required to handle control flow correctly.  On the bright side, if we *do* implement them, I suspect it will solve a bunch of issues all at once.

>> Was there any llvm-dev discussion about this paradigm shift?  I have been very distracted the past couple of months so it might have happened without my noticing or really paying attention to it.  While I think it is probably a net positive in the evolution of debug info for optimized code, I do want to make sure the Usual Suspects are all attuned to it and supportive.  It would also be well worthwhile putting together some words for the description of dbg.value in LangRef about this (which does not have to be part of this patch).
> 
> Zero discussion sorry, I wasn't aware this is a change in interpretation -- however I'm also not aware of any Formal (TM) definition of the semantics of dbg.value intrinsics, particularly with respect to their location in the IR. I've assumed/inferred this is how it's supposed to work from existing code and review feedback.
> 
> Further discussion can't hurt (he said boldly) and getting some meaning written down in the langref would help everyone. I'll draft something and kick that off shortly.

https://llvm.org/docs/SourceLevelDebugging.html#format-common-intrinsics
"This intrinsic provides information when a user source variable is set to a new value."  Naively I read this "when" as meaning, the position of the intrinsic in the instruction stream indicates the starting point where the variable takes on the new value.  But making that clearer would be a Good Thing.
And as various reviews and whatnot fly by, I'm inclined to think it's at least a common misperception that the dbg.value has to be adjacent to the instruction that produces the value operand.  Stating that it's not required would surely help.

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

https://reviews.llvm.org/D58453