[PATCH] D23283: Debugging of optimized code: When locals have their address taken as part of a call use their stack slots as location expressions for debug info.

[Adrian Prantl via llvm-commits]

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

Found one bug, but otherwise this is looking good now, thanks!
LGTM (with all remaining review comments are addressed).


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:2159
@@ +2158,3 @@
+
+  bool isDeref() const {
+    return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_deref;
----------------
Please add a doxygen comment to the function.
Should we rename this to startsWithDeref?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4751
@@ +4750,3 @@
+  SDDbgValue *SDV;
+  auto FISDN = dyn_cast<FrameIndexSDNode>(N.getNode());
+  if (FISDN && Expr->isDeref()) {
----------------
I think we usually write this as "auto *FISDN = ..."

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4758
@@ +4757,3 @@
+    int FI = FISDN->getIndex();
+    SDV = DAG.getFrameIndexDbgValue(Variable, EmptyDIExpr, FI, 0, dl,
+                                    DbgSDNodeOrder);
----------------
After relaxing the condition, you'll want the tail of the dwarf expression here, not just an empty expression.

================
Comment at: test/DebugInfo/AArch64/coalescing.ll:28
@@ -27,3 +27,3 @@
   ; CHECK: .debug_loc contents:
-  ; CHECK: Location description: 70 00
+  ; CHECK: Location description: 8f 0c
   ret void, !dbg !18
----------------
Could you please add a comment with the disassembled dwarf expression?


https://reviews.llvm.org/D23283