[PATCH] D49454: [DebugInfo] LowerDbgDeclare: Add derefs when handling CallInst users

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Thanks so much for seeing this through! This will finally allow us to solve `int x = 0; int *px = &x;`, which has been my favorite optimized debug info test case for a while now. If I filed a bug about it, I can't find it, just https://bugs.llvm.org/show_bug.cgi?id=34136, which has too much discussion on it to really be actionable at this point.



================
Comment at: lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:77
   /// Constructor for frame indices.
-  SDDbgValue(DIVariable *Var, DIExpression *Expr, unsigned FI, DebugLoc dl,
-             unsigned O)
-      : Var(Var), Expr(Expr), DL(std::move(dl)), Order(O), IsIndirect(false) {
-    kind = FRAMEIX;
-    u.FrameIx = FI;
+  static SDDbgValue *CreateFrameIndexDbgValue(SDDbgInfo &DbgInfo,
+                                              DIVariable *Var,
----------------
SDDbgValue creation is kind of an implementation detail of SelectionDAG. I think it might be better to collapse the frame index and virtual register constructor overloads and add a SDDbgValue::DbgValueKind parameter that SelectionDAG::getFrameIndexDbgValue and getVRegDbgValue fill in appropriately.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4965-4971
   if (auto *FISDN = dyn_cast<FrameIndexSDNode>(N.getNode())) {
     // Construct a FrameIndexDbgValue for FrameIndexSDNodes so we can describe
     // stack slot locations as such instead of as indirectly addressed
     // locations.
-    return DAG.getFrameIndexDbgValue(Variable, Expr, FISDN->getIndex(), dl,
-                                     DbgSDNodeOrder);
+    return DAG.getFrameIndexDbgValue(Variable, Expr, FISDN->getIndex(),
+                                     /*IsIndirect*/ false, dl, DbgSDNodeOrder);
   }
----------------
So, this should only be called in the case where the value of the variable is the address of some stack location, i.e. `int x = 0; int *px = &x;`, after optimization, we'll have this direct frame index dbg.value for px. I think that source fragment in a comment would help clarify why IsIndirect is false.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5183
     // the MachineFunction variable table.
     if (FI != std::numeric_limits<int>::max()) {
       if (Intrinsic == Intrinsic::dbg_addr) {
----------------
Yikes, this was easier to read before rL314363. `INT_MAX` is better. =P


https://reviews.llvm.org/D49454