[PATCH] D67398: [DebugInfo] LiveDebugValues: Move DBG_VALUE creation into VarLoc class
Adrian Prantl via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 10 08:50:57 PDT 2019
aprantl added inline comments.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:286
+ const auto &IID = MI.getDesc();
+ auto *Var = MI.getDebugVariable();
+ auto *DIExpr = MI.getDebugExpression();
----------------
nit: here we have enough horizontal space to actually write DILocalVariable and DIExpression, which may make this slightly easier to read.
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:287
+ auto *Var = MI.getDebugVariable();
+ auto *DIExpr = MI.getDebugExpression();
+
----------------
this seems to be used only in one switch case, perhaps sink it?
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:320
+
// Is the Loc field a constant or constant object?
bool isConstant() const { return Kind == ImmediateKind; }
----------------
///
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:360
+ dbgs() << "(null))\n";
+ }
#endif
----------------
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 :-)
================
Comment at: lib/CodeGen/LiveDebugValues.cpp:650
Out << " MI: ";
- VL.dump();
+ VL.dump(TRI);
}
----------------
we should probably pass Out into dump and give a dbgs() default argument?
================
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);
----------------
should we have a self-documenting static constructor function for this case as well?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67398/new/
https://reviews.llvm.org/D67398
More information about the llvm-commits
mailing list