[PATCH] D27468: When GVN removes a redundant load, it should not modify the debug location of the dominating load.

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 6 09:46:20 PST 2016


andreadb created this revision.
andreadb added reviewers: dblaikie, aprantl, probinson, wolfgangp.
andreadb added a subscriber: llvm-commits.

Consider the following example:

  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
  
  define i32 @foo(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 !10
  
  if.then:                                          ; preds = %entry
    %1 = load i32, i32* %Y, align 4, !dbg !11
    %add = add nsw i32 %0, %1, !dbg !12
    call void (...) @fubar() #2, !dbg !13
    br label %if.end, !dbg !14
  
  if.end:                                           ; preds = %if.then, %entry
    %Result.0 = phi i32 [ %add, %if.then ], [ %0, %entry ]
    ret i32 %Result.0, !dbg !15
  }
  
  declare void @fubar(...)

Where:

  !8 = !DILocation(line: 3, column: 16, scope: !6)
  !9 = !DILocation(line: 5, column: 9, scope: !6)
  !10 = !DILocation(line: 5, column: 7, scope: !6)
  !11 = !DILocation(line: 6, column: 15, scope: !6)
  !12 = !DILocation(line: 6, column: 12, scope: !6)
  !13 = !DILocation(line: 7, column: 5, scope: !6)
  !14 = !DILocation(line: 8, column: 3, scope: !6)
  !15 = !DILocation(line: 10, column: 3, scope: !6)

GVN would remove the redundant load from basic block %if.then since it is trivially dominated by an equivalent load from the `entry` block.

The redundant load from %if.then is:

  %1 = load i32, i32* %Y, align 4, !dbg !11

The dominating load from %entry is:

  %0 = load i32, i32* %Y, align 4, !dbg !8

When removing the redundant load %1, GVN also updates the debug location of %0 with the debugloc of %1.

As a result, the load from %entry now incorrectly refers to "line: 6, column: 15" instead of "line: 3,
column: 16".

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 (this is what happens in the example).

Please let me know if okay to commit.

Thanks,
Andrea


https://reviews.llvm.org/D27468

Files:
  lib/Transforms/Scalar/GVN.cpp
  test/Transforms/GVN/dbg-redundant-load.ll


Index: test/Transforms/GVN/dbg-redundant-load.ll
===================================================================
--- test/Transforms/GVN/dbg-redundant-load.ll
+++ 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)
Index: lib/Transforms/Scalar/GVN.cpp
===================================================================
--- lib/Transforms/Scalar/GVN.cpp
+++ lib/Transforms/Scalar/GVN.cpp
@@ -1700,7 +1700,9 @@
     if (isa<PHINode>(V))
       V->takeName(LI);
     if (Instruction *I = dyn_cast<Instruction>(V))
-      if (LI->getDebugLoc())
+      // If we have a fully redundant load LI dominated by a single value I,
+      // then don't update the debug location of I.
+      if (LI->getDebugLoc() && ValuesPerBlock.size() != 1)
         I->setDebugLoc(LI->getDebugLoc());
     if (V->getType()->getScalarType()->isPointerTy())
       MD->invalidateCachedPointerInfo(V);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27468.80436.patch
Type: text/x-patch
Size: 2897 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161206/d1f85f6f/attachment.bin>


More information about the llvm-commits mailing list