[PATCH] D82129: [DebugInfo] Drop location ranges for variables which exist entirely outside the variable's scope

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Paul wrote:

> And yet, the variable was allocated to a register, and the variable's location information pointed to the correct instruction range.
>  Inadequacies in our ability to represent the scope properly shouldn't cause us to eliminate *correct* location information for variables.

My understanding is that we haven't failed to represent the scope here, instead it's been destroyed by optimisation. Here's the MIR before -branch-folder, minus some long lines:

  bb.1.if.then: 
   ; predecessors: %bb.0 
     liveins: $eax 
     DBG_VALUE $eax, $noreg, !"a", !DIExpression(), debug-location !27; t.cpp:11:9 line no:11 
     DBG_VALUE $eax, $noreg, !"a", !DIExpression(), debug-location !34; t.cpp:4:33 @[ t.cpp:12:13 ] line no:4 
     renamable $eax = nsw ADD32ri8 killed renamable $eax(tied-def 0), 1, implicit-def dead $eflags, debug-location !36; t.cpp:5:13 @[ t.cpp:12:13 ] 
     DBG_VALUE $eax, $noreg, !"b", !DIExpression(), debug-location !37; t.cpp:5:7 @ [ t.cpp:12:13 ] line no:5 
     ADD32mi8 $rip, 1, $noreg, @x, $noreg, 1, implicit-def dead $eflags, debug-location !38 :: (deref stuff), (more deref stuff); t.cpp:6:3 @[ t.cpp:12:13 ] 
     DBG_VALUE $eax, $noreg, !"b", !DIExpression(), debug-location !43; t.cpp:12:9 line no:12 
     $ecx = COPY killed renamable $eax, debug-location !44; t.cpp:13:5 
     SEH_Epilogue debug-location !44; t.cpp:13:5 
     $rsp = frame-destroy ADD64ri8 $rsp(tied-def 0), 32, implicit-def dead $eflags, debug-location !44; t.cpp:13:5 
     $rsi = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !44; t.cpp:13:5 
     TCRETURNdi64 @putint, 0, <regmask blah blah blah>, implicit $rsp, implicit $ssp, implicit $ecx, debug-location !44; t.cpp:13:5 
   
   bb.2.if.else: 
   ; predecessors: %bb.0 
     liveins: $eax 
     DBG_VALUE $eax, $noreg, !"c", !DIExpression(), debug-location !46; t.cpp:15:9 line no:15 
     $ecx = COPY killed renamable $eax, debug-location !47; t.cpp:16:5 
     SEH_Epilogue debug-location !47; t.cpp:16:5 
     $rsp = frame-destroy ADD64ri8 $rsp(tied-def 0), 32, implicit-def dead $eflags, debug-location !47; t.cpp:16:5 
     $rsi = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !47; t.cpp:16:5 
     TCRETURNdi64 @putint, 0, <regmask blah blah blah>, implicit $rsp, implicit $ssp, implicit $ecx, debug-location !47; t.cpp:16:5 

And here it is after:

  [More bits of bb.1]
    DBG_VALUE $eax, $noreg, !"b", !DIExpression(), debug-location !37; t.cpp:5:7 @ [ t.cpp:12:13 ] line no:5
    ADD32mi8 $rip, 1, $noreg, @x, $noreg, 1, implicit-def dead $eflags, debug-location !38 :: (deref stuff), (more deref stuff); t.cpp:6:3 @[ t.cpp:12:13 ]
    DBG_VALUE $eax, $noreg, !"b", !DIExpression(), debug-location !43; t.cpp:12:9 line no:12
  
  bb.2.if.else:
  ; predecessors: %bb.0, %bb.1
    liveins: $eax
    DBG_VALUE $eax, $noreg, !"c", !DIExpression(), debug-location !46; t.cpp:15:9 line no:15
    $ecx = COPY killed renamable $eax, debug-location !DILocation(line: 0, scope: !19); t.cpp:0
    SEH_Epilogue debug-location !DILocation(line: 0, scope: !19); t.cpp:0
    $rsp = frame-destroy ADD64ri8 $rsp(tied-def 0), 32, implicit-def dead $eflags, debug-location !DILocation(line: 0, scope: !19); t.cpp:0
    $rsi = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !DILocation(line: 0, scope: !19); t.cpp:0
    TCRETURNdi64 @putint, 0, <regmask blah blah blah>, implicit $rsp, implicit $ssp, implicit $ecx, debug-location !DILocation(line: 0, scope: !19); t.cpp:0

The tail of both blocks have been de-duplicated into the same block (bb.2), from the COPY to ecx, to the tail call. For every pair of duplicate instructions, there was no agreement on the source location, so they've all been given a zero line-number with the scope set to the parent scope of each pair, so for the original source:

  void f(int p) {
     if (p) {
       int a = getint();
       int b = inlineinc(a);
       putint(b);
     } else {
       int c = getint();
       putint(c);
     }
  }

Those instructions are considered in the scope of the "if" rather than for either of the blocks. The patch here then decides that, because the variable location is outside of a scope where it makes sense, it should be deleted. When you say,

> Inadequacies in our ability to represent the scope properly shouldn't cause us to eliminate *correct* location information for variables.

Is there something that we can be doing better to represent the scopes in this scenario? I think it comes back to being able to describe instructions that correspond to multiple source locations, which as far as I'm aware is an open question right now.


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

https://reviews.llvm.org/D82129