[PATCH] D37911: [DebugInfo] Insert DW_OP_deref when spilling indirect DBG_VALUEs

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 15 13:23:27 PDT 2017


aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:112
   const MDNode *Variable;   ///< The debug info variable we are part of.
-  const MDNode *Expression; ///< Any complex address expression.
+  const DIExpression *Expression; ///< Any complex address expression.
   bool IsIndirect;        ///< true if this is a register-indirect+offset value.
----------------
Can you change the Variable type, too, for symmetry?


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:500
   // Get or create the UserValue for (variable,offset).
-  bool IsIndirect = MI.isIndirectDebugValue();
+  bool IsIndirect = MI.getOperand(1).isImm();
   if (IsIndirect)
----------------
Why can't this be API?


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1058
   else
-    BuildMI(*MBB, I, getDebugLoc(), TII.get(TargetOpcode::DBG_VALUE))
-        .add(Loc)
-        .addImm(0U)
-        .addMetadata(Variable)
-        .addMetadata(Expression);
+    MIB.addReg(0U, RegState::Debug);
+  MIB.addMetadata(Variable).addMetadata(Expr);
----------------
There's a much easier to read BuildMI variant that takes an IsIndirect flag.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1071
     unsigned LocNo = I.value();
+    bool Spilled = LocNo != ~0U ? SpilledLocations.test(LocNo) : false;
 
----------------
shall we hide the comparison against ~0U in a self-duocumenting helper function isCondition(LocNo)?


https://reviews.llvm.org/D37911





More information about the llvm-commits mailing list