[PATCH] D38229: [DebugInfo] Insert DEBUG_VALUEs after each register redefinition

[Karl-Johan Karlsson via Phabricator via llvm-commits]

Ka-Ka added a comment.

In https://reviews.llvm.org/D38229#880149, @rnk wrote:

> One concern I have about this is that it may lead to inaccurate debug info when the value is computed long before its (dead) assignment. Consider this kind of program (untested, yet):
>
>   int v1 = getval();
>   int v2 = getval();
>   usevals(v1, v2);
>   v1 = v2;
>   if (v1) { /* empty after inlining, making v2 dead */ }
>
>
> After optimization, v1 will probably use a CSR and v2 will use a volatile register clobbered by the usevals call. So, the dead `v1 = v2` assignment will produce a DBG_VALUE of a physical register that has been clobbered. It would be innacurate to hoist the DBG_VALUE across the `usevals` call, since the value should still be the result of the first `getval` call, not the second.
>
> One way to address this would be to avoid moving DBG_VALUEs across instructions with different DebugLoc lines.
>
> Even if we decide that this issue is unimportant, we should probably write tests for it.


I guess this is the case that you have concern about. Note that this is not actually how the input mir to greedy look like when running the code through llc (due to pr31878). I have edited the mir before feeding it to greedy to get the case that I think you are concerned about.

  bb.0.entry:
    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !14
    dead %eax = MOV32r0 implicit-def dead %eflags, implicit-def %al, debug-location !14
    CALL64pcrel32 @getval, csr_64, implicit %rsp, implicit killed %al, implicit-def %rsp, implicit-def %eax, debug-location !14
    ADJCALLSTACKUP64 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !14
    %2 = COPY killed %eax, debug-location !14
    DBG_VALUE debug-use %2, debug-use _, !11, !DIExpression(), debug-location !15
    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !16
    dead %eax = MOV32r0 implicit-def dead %eflags, implicit-def %al, debug-location !16
    CALL64pcrel32 @getval, csr_64, implicit %rsp, implicit killed %al, implicit-def %rsp, implicit-def %eax, debug-location !16
    ADJCALLSTACKUP64 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !16
    %3 = COPY killed %eax, debug-location !16
    DBG_VALUE debug-use %3, debug-use _, !13, !DIExpression(), debug-location !17
    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !18
    %edi = COPY %2, debug-location !18
    %esi = COPY %3, debug-location !18
    CALL64pcrel32 @usevals, csr_64, implicit %rsp, implicit killed %edi, implicit killed %esi, implicit-def %rsp, debug-location !18
    DBG_VALUE debug-use %3, debug-use _, !11, !DIExpression(), debug-location !15
    ADJCALLSTACKUP64 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit %rsp, debug-location !18
    RET 0, debug-location !19

When looking at the output after livedebugvariables it look like this:

  bb.0.entry:
    liveins: %rbx
  
    frame-setup PUSH64r killed %rbx, implicit-def %rsp, implicit %rsp, debug-location !14
    CFI_INSTRUCTION def_cfa_offset 16
    CFI_INSTRUCTION offset %rbx, -16
    dead %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags, implicit-def %al, debug-location !14
    CALL64pcrel32 @getval, csr_64, implicit %rsp, implicit %al, implicit-def %rsp, implicit-def %eax, debug-location !14
    %ebx = MOV32rr %eax, debug-location !14
    DBG_VALUE debug-use %ebx, debug-use _, !11, !DIExpression(), debug-location !15
    dead %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags, implicit-def %al, debug-location !16
    CALL64pcrel32 @getval, csr_64, implicit %rsp, implicit %al, implicit-def %rsp, implicit-def %eax, debug-location !16
    DBG_VALUE debug-use %eax, debug-use _, !13, !DIExpression(), debug-location !17
    %edi = MOV32rr killed %ebx, debug-location !18
    %esi = MOV32rr killed %eax, debug-location !18
    CALL64pcrel32 @usevals, csr_64, implicit %rsp, implicit killed %edi, implicit killed %esi, implicit-def %rsp, debug-location !18
    DBG_VALUE debug-use %eax, debug-use _, !11, !DIExpression(), debug-location !15
    %rbx = POP64r implicit-def %rsp, implicit %rsp, debug-location !19
    RETQ debug-location !19

The last DBG_VALUE is wrong:

DBG_VALUE debug-use %eax, debug-use _, !11, !DIExpression(), debug-location !15

As I understand your comment this is the situation you are worried about. The value in %eax do not contain any value connected to the variable the DBG_VALUE is suppose to describe as the range ended before the call to `usevals`. The register allocator might have reused eax for another value.

I agree this is a fault, but the output from livedebugvariables shown above is from what is already commited on trunk in svn. The output looks exactly the same with or without the patch in this review. Yes, this is a fault but it is another fault not related to this patch.


https://reviews.llvm.org/D38229