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

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
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);
   }
----------------
vsk wrote:
> rnk wrote:
> > 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.
> Isn't there one more case, where the value of the variable lives inside of some stack location?
> 
> To build on your example, I think the case you described looks like this:
> 
>   dbg.value(i32* %px, !"int *px", !DIExpression())
> 
> And the one I'm pointing out looks like this:
> 
>   dbg.value(i32* %px, !"int x", !DIExpression(DW_OP_deref))
Right, yes, which is the original motivating example for inserting dbg.values before calls.


https://reviews.llvm.org/D49454