[PATCH] D79571: [DebugInfo][DWARF] Emit a single location instead of a location list

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando marked 2 inline comments as done.
Orlando added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1541
+    if (&*I == LScopeEnd)
+      return false;
+
----------------
aprantl wrote:
> Orlando wrote:
> > aprantl wrote:
> > > vsk wrote:
> > > > aprantl wrote:
> > > > > Does this do the right thing if the scope is "interrupted" by another scope within MBB?
> > > > Hm, are you describing a situation like this, where all the code is fit into one MBB?
> > > > 
> > > > ```
> > > > {
> > > >   dbg_value "x"
> > > >   {
> > > >     dbg_value "y"
> > > >     ...
> > > >   }
> > > > }
> > > > ```
> > > > 
> > > > If the inner scope is an inlined frame, I think it'd be ok for the location of 'x' to be valid throughout, since the debugger shouldn't consider 'x' in scope. If its a nested {} scope, 'x' should still be visible within it.
> > > > 
> > > > So I think this is ok. Might be good to add tests for this, though.
> > > I was thinking about
> > > 
> > > ```
> > > MBB:
> > >   insn 1 # scope A
> > >   insn 2 # scope A
> > >   insn 3 # completely unrelated scope B, instruction was moved here by a transformation
> > >   insn 4 # scope A
> > > ...
> > > ```
> > I'm confident that we handle @vsk's example, but less sure about @aprantl's. I'll write up tests for both.
> > 
> > 
> > I'm confident that we handle @vsk's example, but less sure about @aprantl's. I'll write up tests for both.
> 
> What came out of that investigation?
Ah, sorry, I replied to this with a non-inline comment.  I added two more tests, 'single-location-inlined-param.mir' and 'single-location-interrupted-scope.mir', which should cover both of these situations.


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

https://reviews.llvm.org/D79571