[PATCH] D70121: [DebugInfo][LDV] Teach LDV how to identify source variables and handle fragments

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:338
+/// which are live in specific intervals.
+class SourceVariable {
+  using FragmentInfo = DIExpression::FragmentInfo;
----------------
Orlando wrote:
> djtodoro wrote:
> > Orlando wrote:
> > > aprantl wrote:
> > > > Should this use https://reviews.llvm.org/D70486?
> > > The point of `SourceVariable` is to, for any given def, allow us to quickly find
> > > overlapping fragments and explicitly terminate the range of any previous defs.
> > > 
> > > The key difference between `SourceVariable` and `DebugVariable` (from D70486)
> > > is that a `SourceVariable` represents a group of fragments, and a `DebugVariable`
> > > is closer to what I've called a `Fragment` here; it represents a unique _part of_
> > > a variable only.
> > > 
> > > So I don't believe we can use `DebugVariable` in place of `SourceVariable`. But at
> > > the cost of some refactoring and a couple of redundant fields (repeated
> > > `DIVariable*`, `DILocation*`, and bool part of OptFragInfo per `DebugVariable`)
> > > I could replace `Fragment` with `DebugVariable`? I do not feel particularly
> > > strongly either way.
> > > 
> > > The key difference between SourceVariable and DebugVariable (from D70486)
> > > is that a SourceVariable represents a group of fragments, and a DebugVariable
> > > is closer to what I've called a Fragment here; it represents a unique _part of_
> > > a variable only.
> > Can we document this somehow?
> > 
> I could certainly improve the doxygen comment. Would it be appropriate to document anywhere else?
The doxygen comment will help. I am not sure for some other place, maybe the docs/SourceLevelDebugging is good place.


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

https://reviews.llvm.org/D70121