From d2519511ab9c2958c6a8f91c4a5b7e99489f0b5c Mon Sep 17 00:00:00 2001 From: Wolfgang Pieb Date: Wed, 4 Jan 2017 23:58:26 +0000 Subject: [PATCH] [DWARF] Null out the debug locs of load instructions that have been moved by GVN performing partial redundancy elimination (PRE). Not doing so can cause jumpy line tables and confusing (though correct) source attributions. Differential Revision: https://reviews.llvm.org/D27857 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@291037 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/GVN.cpp | 14 ++++++++++++-- test/Transforms/GVN/PRE/phi-translate.ll | 13 ++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp index 9485bfd7c29..0137378b828 100644 --- a/lib/Transforms/Scalar/GVN.cpp +++ b/lib/Transforms/Scalar/GVN.cpp @@ -1572,6 +1572,13 @@ bool GVN::PerformLoadPRE(LoadInst *LI, AvailValInBlkVect &ValuesPerBlock, // 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 @@ bool GVN::PerformLoadPRE(LoadInst *LI, AvailValInBlkVect &ValuesPerBlock, 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, diff --git a/test/Transforms/GVN/PRE/phi-translate.ll b/test/Transforms/GVN/PRE/phi-translate.ll index 42335486910..1f6c7c8d33e 100644 --- a/test/Transforms/GVN/PRE/phi-translate.ll +++ b/test/Transforms/GVN/PRE/phi-translate.ll @@ -4,18 +4,17 @@ target datalayout = "e-p:64:64:64" ; 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 { -- 2.49.0