[PATCH] D128192: [GlobalISel][DebugInfo] Propagate debug location for localized constants

Tue Jul 26 11:13:47 PDT 2022

dblaikie added a comment.

> In D128192#3680110 <https://reviews.llvm.org/D128192#3680110>, @dblaikie wrote:
>
>> so I think backpropagating at the start of a block would address the issue being discussed in this patch.
>
> I can implement that, collect some metrics, but I can't make a decision on this assumption yet. On which stage/pass should such backpropagation happen?

I'm not sure - wherever @probinson implemented the "use zero rather than unspecified location at the start of basic blocks" is probably the place to revisit that and consider backpropagating from the first specified location and see what that looks like.

> In D128192#3680110 <https://reviews.llvm.org/D128192#3680110>, @dblaikie wrote:
>
>> Other zeros would not appear if the constant is put not at the beginning of a block - those would already be getting flow-on locations from the previous locations in a block.
>
> We can consider this patch as changing a propagation direction for constants. From forward propagation, the default debugger behavior, to backward propagation.

Slight pedantry: This isn't a debugger behavior thing, it's a DWARF format thing (so a bit more robust/explicitly guaranteed/required than something some debuggers choose to do) - a source line in the line table is valid from the address it's specified at until the next point in the line table that changes the line/address.

> I think it affects not only instructions at the beginning of the basic block, since changing the propagation direction may make boundaries between source code lines more precise, and thus, improve stepping behavior (https://github.com/llvm/llvm-project/issues/56370 addressing such cases too).

Yeah, I'm not entirely understanding that issue and/or the connection with this one - some complications/lots of nuanced details in this area for sure.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128192