[llvm] [DebugInfo][RemoveDIs] Don't convert debug-intrinsics to Unreachable (PR #72380)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 15 05:22:37 PST 2023
https://github.com/jmorse created https://github.com/llvm/llvm-project/pull/72380
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.
>From c0a695ff145e2bb0d2d8783502cf5827e625ed50 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 22 Jun 2023 11:13:20 +0100
Subject: [PATCH] [DebugInfo][RemoveDIs] Don't convert debug-intrinsics to
Unreachable
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.
---
llvm/lib/Transforms/IPO/SCCP.cpp | 4 +--
.../InstCombine/InstructionCombining.cpp | 6 ++++
llvm/lib/Transforms/Utils/Local.cpp | 5 +--
llvm/test/Transforms/InstCombine/assume.ll | 3 ++
.../unreachable-dbg-info-modified.ll | 1 +
.../SCCP/ipsccp-branch-unresolved-undef.ll | 35 +++++++++++++++++++
.../SimplifyCFG/PR27615-simplify-cond-br.ll | 3 ++
.../Transforms/SimplifyCFG/branch-fold-dbg.ll | 1 +
llvm/test/Transforms/SimplifyCFG/dbginfo.ll | 1 +
9 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index 8388354d6e6ac85c..b1f9b827dcbaf3c7 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 2e0b185769041cc2..8988d658f9c22771 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 aacf66bfe38eb913..0d325093ee2dca40 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 934e9594f3f7b5ad..0ae558c6a21da603 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 c0fedc624eaef9e7..7b88b46e22af2f74 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 ab4bfa59f5b0040b..feb6d127df9619e7 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 e64d970ad70ec309..e24e239323cda507 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 41486c5935a392c7..e754b5187d87e604 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 cce72977d6605266..d142eb2db8ae28c2 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 }
More information about the llvm-commits
mailing list