[PATCH] D46129: [SelectionDAG] Improve selection of DBG_VALUE using a PHI node result

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: test/DebugInfo/COFF/pieces.ll:46
+; ASM: [[oy_ox_start:\.Ltmp[0-9]+]]:
 ; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
+; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
----------------
bjope wrote:
> aprantl wrote:
> > I'm having a hard time interpreting the assembler comment. Does this mean that the range of the DBG_VALUE get's immediately terminated by the assignment to edi, or does that mean "starting with the assignment"?
> The original loop in C looks like this (at least according to the comment in the test case, I've not regenerated it from C since it is an old test case):
> ```
> ;   for (i = 0; i < n; i++) {
> ;     o.x = g(o.x);
> ;     o.y = g(o.y);
> ;   }
> ;   return o.x + o.y;
> ```
> 
> Without the patch we get this kind of structure in the loop body:
> 
> ```
> ox_start:
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
>   o.x = g(o.x);
> oy_start:
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
>   o.y = g(o.y);
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
> oy_end:
> ```
> 
> after the patch we get
> 
> ```
> ox_oy_start:
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
>   o.x = g(o.x);
> ox_start:
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
>   o.y = g(o.y);
> oy_start:
>   #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
> oy_end:
> ```
> So the difference is basically that we now track "o.y" also for the first part of the loop body. That should be an improvement!
> 
> Perhaps I should rename ox_start as ox_restart, and oy_start as oy_restart, to indicate that those labels indicate where a new live range starts for the DEBUG_VALUEs (after the updates of o.x and o.y)? Or we could just call them LBL1, LBL2, LBL3 and LBL4.
> 
> 
I believe your analysis is correct! This is great, I was really annoyed that we didn't track `o.y` for the first part of the loop when I wrote this test case initially. I used it for a demo and I had to carefully put my breakpoint on the loop backedge so that both parts would show up in the debugger.


Repository:
  rL LLVM

https://reviews.llvm.org/D46129