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

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

There's a lot going on in the explanation, and partly I'm handicapped by lack of familiarity with the picky details of LLVM's location-list construction.  Given all the attention to where dbg.value instructions are placed, it seems like placement is a significant factor in that construction.  Intuitively, placing the dbg.value near the instruction that defines the value (assuming there is such an instruction) allows the location-list to better describe the instruction stream that is actually emitted.  And, that's really what I want from a location list.
I would caution against focusing too much on the coverage statistics.  As you say, what we really want to know is "whether the variable locations are defined over instructions in the program that make sense" and the statistics don't tell us that.
So, what instructions "make sense" in a location list? Well, the ones where the variable has an identifiable machine location or constant value.  If optimization causes a value to be defined "before" the variable is declared, that does not really worry me.  The goal here is to produce a correct description of the instructions-actually-emitted.
"One example problem is that conditional variable assignments can be hoisted and appear to debuggers unconditionally." Are the values actually defined unconditionally? Then it is correct to describe them that way to the debugger.  I assume this is referring to PR38754.  If the instructions to compute the value are hoisted, my assertion is that the location-list should describe the variable with that value when it actually occurs.
In my view, the real problem as stated in the PR appears to be that the value later gets *lost*, sadly before the instructions directly related to the switch-case where the variable is used; the real problem is *not* that the value's description starts too soon.

So given all that, how does this patch help address the real problem?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58453