[PATCH] D23715: Add @llvm.dbg.value entries for the phi node created by -mem2reg

[David Blaikie via llvm-commits]

dblaikie added inline comments.

================
Comment at: lib/Transforms/Utils/Local.cpp:1066-1078
@@ +1065,15 @@
+  // not inserting the same dbg.value intrinsic over and over.
+  llvm::BasicBlock::InstListType::iterator NextI(I);
+  while (NextI != I->getParent()->getInstList().end()) {
+    ++NextI;
+    if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(NextI)) {
+      if (DVI->getValue() == I &&
+          DVI->getOffset() == 0 &&
+          DVI->getVariable() == DIVar &&
+          DVI->getExpression() == DIExpr)
+        return true;
+    } else
+      return false;
+  }
+  return false;
+}
----------------
This might be better written as an llvm::any_of (or at least llvm::find_if), perhaps?

================
Comment at: lib/Transforms/Utils/Local.cpp:1067-1069
@@ +1066,5 @@
+  llvm::BasicBlock::InstListType::iterator NextI(I);
+  while (NextI != I->getParent()->getInstList().end()) {
+    ++NextI;
+    if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(NextI)) {
+      if (DVI->getValue() == I &&
----------------
This looks like it might try to dyn_cast the end iterator, which I would imagine isn't valid in general? (is there a reason it's incrementing first then doing work, rather than a more normal for (even range-for) loop?)

================
Comment at: test/CodeGen/AArch64/phi-dbg.ll:1-4
@@ +1,5 @@
+; RUN: llc -O0 %s -mtriple=aarch64 -o - | FileCheck %s
+
+; Test that a DEBUG_VALUE node is create for variable c after the phi has been
+; converted to a ldr.    The DEBUG_VALUE must be *after* the ldr and not before it.
+
----------------
Seems like this patch could be split in two - this test and the associated improvements to lib/CodeGen - and, separately, the changes mem2reg (& the other test) mentioned in the description.


https://reviews.llvm.org/D23715