[PATCH] D70121: [WIP][DebugInfo][LDV] Attempt to teach LDV how to handle fragments

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:322
+/// A user value group contains the set of debug info user variables
+/// which refer to the same DILocalVariable in the same inlining scope.
+class UserValueGroup {
----------------
Orlando wrote:
> Orlando wrote:
> > aprantl wrote:
> > > I don't know if this helps, but in other places we conceptually treat `std::pair<DILocalVariable *, FragmentInfo>` as unique "variables" and the remainder expression as part of a "location".
> > Yes that makes sense, I can probably improve the class name and comment here.
> These terms feel overloaded but much more descriptive. How do you feel about this kind of thing:
> 
> UserValue is now responsible for all DBG_VALUEs which refer to an exact _fragment_.
> 
>     UserValue -> Fragment
> 
> UserValueGroup groups UserValues (Fragments) that refer to the same _variable_
> 
>     UserValueGroup -> Variable
> 
> Maybe a suitable prefix would help too.
> UserValue is now responsible for all DBG_VALUEs which refer to an exact _fragment_.

You need to be prepared to deal with debug info were the fragments aren't always the same size. Particularly it's common to have both the entire variable and then a version of the variable being split up into fragments. In other passes I've used a concept of a non-overlapping fragment, which I believe is what you mean, but I wanted to be sure you are aware of this!



================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:132
 private:
   unsigned LocNo;
+  const DIExpression *Expression;
----------------
`= 0`


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:133
   unsigned LocNo;
+  const DIExpression *Expression;
 };
----------------
`= nullptr`


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

https://reviews.llvm.org/D70121