[PATCH] D27857: [PATCH][DWARF] Don't propagate or set debug locations for PRE loads and associated address calculations

[Wolfgang Pieb via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL291037: [DWARF] Null out the debug locs of load instructions that have been moved by… (authored by wolfgangp).

Changed prior to commit:
  https://reviews.llvm.org/D27857?vs=81778&id=83159#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27857

Files:
  llvm/trunk/lib/Transforms/Scalar/GVN.cpp
  llvm/trunk/test/Transforms/GVN/PRE/phi-translate.ll


Index: llvm/trunk/test/Transforms/GVN/PRE/phi-translate.ll
===================================================================
--- llvm/trunk/test/Transforms/GVN/PRE/phi-translate.ll
+++ llvm/trunk/test/Transforms/GVN/PRE/phi-translate.ll
@@ -4,18 +4,17 @@
 
 ; CHECK-LABEL: @foo(
 ; CHECK: entry.end_crit_edge:
-; CHECK:   %j.phi.trans.insert = sext i32 %x to i64, !dbg [[J_LOC:![0-9]+]]
-; CHECK:   %q.phi.trans.insert = getelementptr {{.*}}, !dbg [[Q_LOC:![0-9]+]]
-; CHECK:   %n.pre = load i32, i32* %q.phi.trans.insert, !dbg [[N_LOC:![0-9]+]]
+; CHECK: %[[INDEX:[a-z0-9.]+]] = sext i32 %x to i64{{$}}
+; CHECK: %[[ADDRESS:[a-z0-9.]+]] = getelementptr [100 x i32], [100 x i32]* @G, i64 0, i64 %[[INDEX]]{{$}}
+; CHECK:   %n.pre = load i32, i32* %[[ADDRESS]]{{$}}
+; CHECK: br label %end
 ; CHECK: then:
 ; CHECK:   store i32 %z
 ; CHECK: end:
-; CHECK:   %n = phi i32 [ %n.pre, %entry.end_crit_edge ], [ %z, %then ], !dbg [[N_LOC]]
+; CHECK:   %n = phi i32 [ %n.pre, %entry.end_crit_edge ], [ %z, %then ], !dbg [[N_LOC:![0-9]+]]
 ; CHECK:   ret i32 %n
 
-; CHECK-DAG: [[J_LOC]] = !DILocation(line: 45, column: 1, scope: !{{.*}})
-; CHECK-DAG: [[Q_LOC]] = !DILocation(line: 46, column: 1, scope: !{{.*}})
-; CHECK-DAG: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}})
+; CHECK: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}})
 
 @G = external global [100 x i32]
 define i32 @foo(i32 %x, i32 %z) !dbg !6 {
Index: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
===================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp
@@ -1572,6 +1572,13 @@
 
   // Assign value numbers to the new instructions.
   for (Instruction *I : NewInsts) {
+    // Instructions that have been inserted in predecessor(s) to materialize
+    // the load address do not retain their original debug locations. Doing
+    // so could lead to confusing (but correct) source attributions.
+    // FIXME: How do we retain source locations without causing poor debugging
+    // behavior?
+    I->setDebugLoc(DebugLoc());
+
     // FIXME: We really _ought_ to insert these value numbers into their
     // parent's availability map.  However, in doing so, we risk getting into
     // ordering issues.  If a block hasn't been processed yet, we would be
@@ -1601,8 +1608,11 @@
     if (auto *RangeMD = LI->getMetadata(LLVMContext::MD_range))
       NewLoad->setMetadata(LLVMContext::MD_range, RangeMD);
 
-    // Transfer DebugLoc.
-    NewLoad->setDebugLoc(LI->getDebugLoc());
+    // We do not propagate the old load's debug location, because the new
+    // load now lives in a different BB, and we want to avoid a jumpy line
+    // table.
+    // FIXME: How do we retain source locations without causing poor debugging
+    // behavior?
 
     // Add the newly created load.
     ValuesPerBlock.push_back(AvailableValueInBlock::get(UnavailablePred,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27857.83159.patch
Type: text/x-patch
Size: 2940 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170105/a9605c50/attachment.bin>