[PATCH] D100165: [GVNHoist] fix a bug where GVNHoist preserves the debug location when it should be dropped

[Yuanbo Li via Phabricator via llvm-commits]

yuanboli233 created this revision.
yuanboli233 added reviewers: jmorse, dblaikie.
Herald added a subscriber: hiraditya.
yuanboli233 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

It is a proposed patch for the bug presented here: https://bugs.llvm.org/show_bug.cgi?id=49099

In LLVM debug information update guide, "Hoisting identical instructions which appear in several successor blocks into a predecessor block. In this case, there is no single merged instruction. The rule for dropping locations applies."

Here in the code, the hoisted instruction keeps the previous debug location. If there are multiple of the instructions hoisted, then a conservative approach is possibly dropping the debug location.

I also provide a test case here.

If the patch makes sense, can you help commit it? Thanks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100165

Files:
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/test/Transforms/GVNHoist/hoist_debugloc.ll


Index: llvm/test/Transforms/GVNHoist/hoist_debugloc.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/GVNHoist/hoist_debugloc.ll
@@ -0,0 +1,58 @@
+; RUN: opt -gvn-hoist %s -S 2>&1 | FileCheck %s
+; CHECK: store i32 1, i32* @{{[a-zA-Z_][a-zA-Z0-9_]*}}, align 4
+source_filename = "abc.ll"
+
+ at G = external global i32, align 4
+
+define void @foo(i32 %c1) !dbg !6 {
+entry:
+  call void @llvm.dbg.value(metadata i32 0, metadata !9, metadata !DIExpression()), !dbg !11
+  switch i32 %c1, label %exit1 [
+    i32 0, label %sw0
+    i32 1, label %sw1
+  ], !dbg !11
+
+sw0:                                              ; preds = %entry
+  store i32 1, i32* @G, align 4, !dbg !12
+  br label %exit, !dbg !13
+
+sw1:                                              ; preds = %entry
+  store i32 1, i32* @G, align 4, !dbg !14
+  br label %exit, !dbg !15
+
+exit1:                                            ; preds = %entry
+  store i32 1, i32* @G, align 4, !dbg !16
+  ret void, !dbg !17
+
+exit:                                             ; preds = %sw1, %sw0
+  ret void, !dbg !18
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+attributes #0 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!3, !4}
+!llvm.module.flags = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "abc.ll", directory: "/")
+!2 = !{}
+!3 = !{i32 8}
+!4 = !{i32 1}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+!7 = !DISubroutineType(types: !2)
+!8 = !{!9}
+!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
+!11 = !DILocation(line: 1, column: 1, scope: !6)
+!12 = !DILocation(line: 2, column: 1, scope: !6)
+!13 = !DILocation(line: 3, column: 1, scope: !6)
+!14 = !DILocation(line: 4, column: 1, scope: !6)
+!15 = !DILocation(line: 5, column: 1, scope: !6)
+!16 = !DILocation(line: 6, column: 1, scope: !6)
+!17 = !DILocation(line: 7, column: 1, scope: !6)
+!18 = !DILocation(line: 8, column: 1, scope: !6)
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -1140,6 +1140,10 @@
       // Move the instruction at the end of HoistPt.
       Instruction *Last = DestBB->getTerminator();
       MD->removeInstruction(Repl);
+      // if there are multiple instructions to hoist, we should conservatively drop the debug location: according to the LLVM debug information update guide, if multiple identical instructions are hoisted into a predecessor block, the debug information should not be preserved.
+      if(InstructionsToHoist.size() > 1){
+        Repl->dropLocation();
+      }
       Repl->moveBefore(Last);
 
       DFSNumber[Repl] = DFSNumber[Last]++;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100165.336328.patch
Type: text/x-patch
Size: 3326 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210409/d880d6a7/attachment.bin>