From: Andrea Di Biagio <Andrea_DiBiagio@sn.scee.net>
Date: Wed, 7 Dec 2016 12:31:36 +0000 (+0000)
Subject: When GVN removes a redundant load, it should not modify the debug location of the... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4632c8a6b8d944c1cb81b26ab88959fcc37be783;p=llvm

When GVN removes a redundant load, it should not modify the debug location of the dominating load.

In the case of a fully redundant load LI dominated by an equivalent load V, GVN
should always preserve the original debug location of V. Otherwise, we risk to
introduce an incorrect stepping.
If V has debug info, then clearly it should not be modified. If V has a null
debugloc, then it is still potentially incorrect to propagate LI's debugloc
because LI may not post-dominate V.

Differential Revision: https://reviews.llvm.org/D27468


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@288903 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
index 81f273ff318..9485bfd7c29 100644
--- a/lib/Transforms/Scalar/GVN.cpp
+++ b/lib/Transforms/Scalar/GVN.cpp
@@ -1700,7 +1700,10 @@ bool GVN::processNonLocalLoad(LoadInst *LI) {
     if (isa<PHINode>(V))
       V->takeName(LI);
     if (Instruction *I = dyn_cast<Instruction>(V))
-      if (LI->getDebugLoc())
+      // If instruction I has debug info, then we should not update it.
+      // Also, if I has a null DebugLoc, then it is still potentially incorrect
+      // to propagate LI's DebugLoc because LI may not post-dominate I.
+      if (LI->getDebugLoc() && ValuesPerBlock.size() != 1)
         I->setDebugLoc(LI->getDebugLoc());
     if (V->getType()->getScalarType()->isPointerTy())
       MD->invalidateCachedPointerInfo(V);
diff --git a/test/Transforms/GVN/dbg-redundant-load.ll b/test/Transforms/GVN/dbg-redundant-load.ll
new file mode 100644
index 00000000000..8e5a48b14a5
--- /dev/null
+++ b/test/Transforms/GVN/dbg-redundant-load.ll
@@ -0,0 +1,52 @@
+; RUN: opt -gvn -S < %s | FileCheck %s
+
+; Check that the redundant load from %if.then is removed.
+; Also, check that the debug location associated to load %0 still refers to
+; line 3 and not line 6.
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; CHECK: @test_redundant_load(
+; CHECK-LABEL: entry:
+; CHECK-NEXT: load i32, i32* %Y, align 4, !dbg ![[LOC:[0-9]+]]
+; CHECK-LABEL: if.then:
+; CHECK-NOT: load
+; CHECK-LABEL: if.end:
+; CHECK: ![[LOC]] = !DILocation(line: 3, scope: !{{.*}})
+
+define i32 @test_redundant_load(i32 %X, i32* %Y) !dbg !6 {
+entry:
+  %0 = load i32, i32* %Y, align 4, !dbg !8
+  %cmp = icmp sgt i32 %X, -1, !dbg !9
+  br i1 %cmp, label %if.then, label %if.end, !dbg !9
+
+if.then:                                          ; preds = %entry
+  %1 = load i32, i32* %Y, align 4, !dbg !10
+  %add = add nsw i32 %0, %1, !dbg !10
+  call void @foo(), !dbg !11
+  br label %if.end, !dbg !12
+
+if.end:                                           ; preds = %if.then, %entry
+  %Result.0 = phi i32 [ %add, %if.then ], [ %0, %entry ]
+  ret i32 %Result.0, !dbg !13
+}
+
+declare void @foo()
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "test.cpp", directory: "")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"PIC Level", i32 2}
+!6 = distinct !DISubprogram(name: "test_redundant_load", scope: !1, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 3, scope: !6)
+!9 = !DILocation(line: 5, scope: !6)
+!10 = !DILocation(line: 6, scope: !6)
+!11 = !DILocation(line: 7, scope: !6)
+!12 = !DILocation(line: 8, scope: !6)
+!13 = !DILocation(line: 10, scope: !6)