[PATCH] D67398: [DebugInfo] LiveDebugValues: Move DBG_VALUE creation into VarLoc class
Jeremy Morse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 11 09:53:29 PDT 2019
jmorse added inline comments.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:262
+ VarLoc VL(MI, LS);
+ VL.Kind = EntryValueKind;
+ VL.Expr = EntryExpr;
----------------
djtodoro wrote:
> Is this redundant, since we already set the Kind within the `VarLoc`'s constructor?
This is deliberately building a VarLoc from the register location in `MI`, then modifying it to be an entry value.
This could be done differently, but I thought a function that "turns this existing DBG_VALUE into an entry-value VarLoc" would be simpler than building a whole VarLoc from scratch. I'll update the comment to indicate that's what I intended.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:287
+ auto *Var = MI.getDebugVariable();
+ auto *DIExpr = MI.getDebugExpression();
+
----------------
aprantl wrote:
> this seems to be used only in one switch case, perhaps sink it?
It's passed to BuildMI in each switch case (I've moved EntryValueKind to not fall-through with the latest revision).
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:360
+ dbgs() << "(null))\n";
+ }
#endif
----------------
aprantl wrote:
> It would be nice if this also printed the kind that we constructed it as (CopyLoc, ...) maybe using an asserts-only kind field? Maybe this also isn't necessary :-)
Ah well, determining where a VarLoc originates from is the topic of a future patch, best leave it til then when it'll be easier to implement.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:780
VarLoc::SpillLoc SpillLocation = extractSpillBaseRegAndOffset(MI);
- auto *SpillExpr = DIExpression::prepend(DebugInstr->getDebugExpression(),
- DIExpression::ApplyOffset,
- SpillLocation.SpillOffset);
- NewDebugInstr = BuildMI(
- *MF, DebugInstr->getDebugLoc(), DebugInstr->getDesc(), true,
- SpillLocation.SpillBase, DebugInstr->getDebugVariable(), SpillExpr);
- VarLoc VL(*NewDebugInstr, SpillLocation.SpillBase,
- SpillLocation.SpillOffset, LS, *DebugInstr);
- ProcessVarLoc(VL, NewDebugInstr);
- LLVM_DEBUG(dbgs() << "Creating DBG_VALUE inst for spill: ";
- NewDebugInstr->print(dbgs(), /*IsStandalone*/false,
- /*SkipOpers*/false, /*SkipDebugLoc*/false,
- /*AddNewLine*/true, TII));
+ VarLoc VL(*DebugInstr, SpillLocation.SpillBase,
+ SpillLocation.SpillOffset, LS);
----------------
aprantl wrote:
> should we have a self-documenting static constructor function for this case as well?
Sounds good
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67398/new/
https://reviews.llvm.org/D67398
More information about the llvm-commits
mailing list