[llvm] [DebugInfo][RemoveDIs] Don't convert debug-intrinsics to Unreachable (PR #72380)

via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 15 05:22:54 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

<details>
<summary>Changes</summary>

It might seem obvious, but it's not a good idea to convert a debug-intrinsic instruction into an UnreachableInst, as this means things operate differently with and without the -g option. However this can happen due to the "mutate the next instruction" API calls we make. With RemoveDIs eliminating debug intrinsics, this behaviour is at risk of changing, hence this patch ensures we only ever mutate the next _non_ debuginfo instruction into an Unreachable.

The tests instrumented with the --try... flag all exercise this, I've added some metadata to a SCCP test to ensure it's exercised.

---
Full diff: https://github.com/llvm/llvm-project/pull/72380.diff


9 Files Affected:

- (modified) llvm/lib/Transforms/IPO/SCCP.cpp (+2-2) 
- (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+6) 
- (modified) llvm/lib/Transforms/Utils/Local.cpp (+3-2) 
- (modified) llvm/test/Transforms/InstCombine/assume.ll (+3) 
- (modified) llvm/test/Transforms/InstCombine/unreachable-dbg-info-modified.ll (+1) 
- (modified) llvm/test/Transforms/SCCP/ipsccp-branch-unresolved-undef.ll (+35) 
- (modified) llvm/test/Transforms/SimplifyCFG/PR27615-simplify-cond-br.ll (+3) 
- (modified) llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll (+1) 
- (modified) llvm/test/Transforms/SimplifyCFG/dbginfo.ll (+1) 


``````````diff
diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index 8388354d6e6ac85..b1f9b827dcbaf3c 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -236,11 +236,11 @@ static bool runIPSCCP(
     // nodes in executable blocks we found values for. The function's entry
     // block is not part of BlocksToErase, so we have to handle it separately.
     for (BasicBlock *BB : BlocksToErase) {
-      NumInstRemoved += changeToUnreachable(BB->getFirstNonPHI(),
+      NumInstRemoved += changeToUnreachable(BB->getFirstNonPHIOrDbg(),
                                             /*PreserveLCSSA=*/false, &DTU);
     }
     if (!Solver.isBlockExecutable(&F.front()))
-      NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHI(),
+      NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHIOrDbg(),
                                             /*PreserveLCSSA=*/false, &DTU);
 
     BasicBlock *NewUnreachableBB = nullptr;
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 2e0b185769041cc..8988d658f9c2277 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2866,10 +2866,16 @@ void InstCombinerImpl::handleUnreachableFrom(
     }
     if (Inst.isEHPad() || Inst.getType()->isTokenTy())
       continue;
+    // RemoveDIs: erase debug-info on this instruction manually.
+    Inst.dropDbgValues();
     eraseInstFromFunction(Inst);
     MadeIRChange = true;
   }
 
+  // RemoveDIs: to match behaviour in dbg.value mode, drop debug-info on
+  // terminator too.
+  BB->getTerminator()->dropDbgValues();
+
   // Handle potentially dead successors.
   for (BasicBlock *Succ : successors(BB))
     addDeadEdge(BB, Succ, Worklist);
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index aacf66bfe38eb91..0d325093ee2dca4 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2432,6 +2432,7 @@ unsigned llvm::changeToUnreachable(Instruction *I, bool PreserveLCSSA,
       Updates.push_back({DominatorTree::Delete, BB, UniqueSuccessor});
     DTU->applyUpdates(Updates);
   }
+  BB->flushTerminatorDbgValues();
   return NumInstrsRemoved;
 }
 
@@ -2585,9 +2586,9 @@ static bool markAliveBlocks(Function &F,
           // If we found a call to a no-return function, insert an unreachable
           // instruction after it.  Make sure there isn't *already* one there
           // though.
-          if (!isa<UnreachableInst>(CI->getNextNode())) {
+          if (!isa<UnreachableInst>(CI->getNextNonDebugInstruction())) {
             // Don't insert a call to llvm.trap right before the unreachable.
-            changeToUnreachable(CI->getNextNode(), false, DTU);
+            changeToUnreachable(CI->getNextNonDebugInstruction(), false, DTU);
             Changed = true;
           }
           break;
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index 934e9594f3f7b5a..0ae558c6a21da60 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -2,6 +2,9 @@
 ; RUN: opt < %s -passes=instcombine -S | FileCheck --check-prefixes=CHECK,DEFAULT %s
 ; RUN: opt < %s -passes=instcombine --enable-knowledge-retention -S | FileCheck --check-prefixes=CHECK,BUNDLES %s
 
+; RUN: opt < %s -passes=instcombine -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK,DEFAULT %s
+; RUN: opt < %s -passes=instcombine --enable-knowledge-retention -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK,BUNDLES %s
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
diff --git a/llvm/test/Transforms/InstCombine/unreachable-dbg-info-modified.ll b/llvm/test/Transforms/InstCombine/unreachable-dbg-info-modified.ll
index c0fedc624eaef9e..7b88b46e22af2f7 100644
--- a/llvm/test/Transforms/InstCombine/unreachable-dbg-info-modified.ll
+++ b/llvm/test/Transforms/InstCombine/unreachable-dbg-info-modified.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; When removing the llvm.dbg.value intrinsic in the unreachable block
 ; InstCombine would incorrectly return a false Modified status.
diff --git a/llvm/test/Transforms/SCCP/ipsccp-branch-unresolved-undef.ll b/llvm/test/Transforms/SCCP/ipsccp-branch-unresolved-undef.ll
index ab4bfa59f5b0040..feb6d127df9619e 100644
--- a/llvm/test/Transforms/SCCP/ipsccp-branch-unresolved-undef.ll
+++ b/llvm/test/Transforms/SCCP/ipsccp-branch-unresolved-undef.ll
@@ -1,5 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
 ; RUN: opt < %s -S -passes=ipsccp | FileCheck %s
+; RUN: opt < %s -S -passes=ipsccp --try-experimental-debuginfo-iterators | FileCheck %s
+;; Check that @patatino is optimised to "unreachable" given that it branches on
+;; undef. Check too that debug intrinsics have no effect on this.
 
 define void @main() {
 ; CHECK-LABEL: define {{[^@]+}}@main() {
@@ -17,7 +20,39 @@ define internal i1 @patatino(i1 %a) {
 ;
   br i1 %a, label %ontrue, label %onfalse
 ontrue:
+  call void @llvm.dbg.value(metadata i32 0, metadata !6, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.value(metadata i32 1, metadata !9, metadata !DIExpression()), !dbg !11
   ret i1 false
 onfalse:
+  call void @llvm.dbg.value(metadata i32 0, metadata !6, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.value(metadata i32 1, metadata !9, metadata !DIExpression()), !dbg !11
   ret i1 false
 }
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) nounwind readnone
+
+!llvm.module.flags = !{!21}
+!llvm.dbg.cu = !{!2}
+
+!0 = distinct !DISubprogram(name: "foo", line: 2, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !2, file: !20, scope: !1, type: !3)
+!1 = !DIFile(filename: "b.c", directory: "/private/tmp")
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang", isOptimized: true, emissionKind: FullDebug, file: !20)
+!3 = !DISubroutineType(types: !4)
+!4 = !{!5}
+!5 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!6 = !DILocalVariable(name: "i", line: 2, arg: 1, scope: !0, file: !1, type: !5)
+!7 = !DILocation(line: 2, column: 13, scope: !0)
+!9 = !DILocalVariable(name: "k", line: 3, scope: !10, file: !1, type: !5)
+!10 = distinct !DILexicalBlock(line: 2, column: 16, file: !20, scope: !0)
+!11 = !DILocation(line: 3, column: 12, scope: !10)
+!12 = !DILocation(line: 4, column: 3, scope: !10)
+!13 = !DILocation(line: 5, column: 5, scope: !14)
+!14 = distinct !DILexicalBlock(line: 4, column: 10, file: !20, scope: !10)
+!15 = !DILocation(line: 6, column: 3, scope: !14)
+!16 = !DILocation(line: 7, column: 5, scope: !17)
+!17 = distinct !DILexicalBlock(line: 6, column: 10, file: !20, scope: !10)
+!18 = !DILocation(line: 8, column: 3, scope: !17)
+!19 = !DILocation(line: 9, column: 3, scope: !10)
+!20 = !DIFile(filename: "b.c", directory: "/private/tmp")
+!21 = !{i32 1, !"Debug Info Version", i32 3}
+
diff --git a/llvm/test/Transforms/SimplifyCFG/PR27615-simplify-cond-br.ll b/llvm/test/Transforms/SimplifyCFG/PR27615-simplify-cond-br.ll
index e64d970ad70ec30..e24e239323cda50 100644
--- a/llvm/test/Transforms/SimplifyCFG/PR27615-simplify-cond-br.ll
+++ b/llvm/test/Transforms/SimplifyCFG/PR27615-simplify-cond-br.ll
@@ -1,6 +1,9 @@
 ; RUN: opt -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -strip-debug < %s | FileCheck %s
 ; RUN: opt -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 < %s | FileCheck %s
 
+; RUN: opt -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -strip-debug < %s --try-experimental-debuginfo-iterators | FileCheck %s
+; RUN: opt -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 < %s --try-experimental-debuginfo-iterators | FileCheck %s
+
 ; Test case for BUG-27615
 ; Test that simplify cond branch produce same result for debug and non-debug builds
 ; CHECK: select i1 %or.cond, i32 -1, i32 5
diff --git a/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll b/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
index 41486c5935a392c..e754b5187d87e60 100644
--- a/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
+++ b/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck %s
+; RUN: opt -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 %0 = type { ptr, ptr }
 
diff --git a/llvm/test/Transforms/SimplifyCFG/dbginfo.ll b/llvm/test/Transforms/SimplifyCFG/dbginfo.ll
index cce72977d660526..d142eb2db8ae28c 100644
--- a/llvm/test/Transforms/SimplifyCFG/dbginfo.ll
+++ b/llvm/test/Transforms/SimplifyCFG/dbginfo.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S --try-experimental-debuginfo-iterators | FileCheck %s
 
   %llvm.dbg.anchor.type = type { i32, i32 }
   %llvm.dbg.basictype.type = type { i32, ptr, ptr, ptr, i32, i64, i64, i64, i32, i32 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/72380


More information about the llvm-commits mailing list