[llvm] [DebugInfo] Clone dbg.values in SimplifyCFG like normal instructions (PR #72526)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 16 09:34:47 PST 2023
https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/72526
>From a31c7e1c92d6d1f47922f4df0e64aac75b468998 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 16 Nov 2023 15:41:52 +0000
Subject: [PATCH 1/3] [DebugInfo] Clone dbg.values in SimplifyCFG like normal
instructions
The code in the CloneInstructionsIntoPredec... function modified by this
patch has a long history that dates back to 2011, see d715ec82b4ad12c59.
There, when folding branches, all dbg.value intrinsics seen when folding
would be saved and then re-inserted at the end of whatever was folded. Over
the last 12 years this behaviour has been preserved.
However, IMO it's bad behaviour. If we have:
inst1
dbg.value1
inst2
dbg.value2
And we fold that sequence into a different block, then we would want the
instructions and variable assignments to appear in the same order. However
because of this old behaviour, the dbg.values are sunk, and we get:
inst1
inst2
dbg.value1
dbg.value2
This clustering of dbg.values can make assignments to the same variable
invisible, as well as reducing the coverage of other assignments.
This patch relaxes the CloneInstructions... function and allows it to clone
and update dbg.values in-place, causing them to appear in the original
order in the destination block. I've added some extra dbg.values to the
updated test: without the changes to the pass, the dbg.values sink into a
blob ahead of the select.
(Metadata changes to make the LLVM-IR parser not drop the debug-info for it
being out of date. The RemoveDIs related RUN line has been removed because
it was spuriously passing due to the debug-info being dropped).
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 18 ++++------
.../Transforms/SimplifyCFG/branch-fold-dbg.ll | 34 ++++++++++++-------
2 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 4dae52a8ecffdf6..401a71d59f3324e 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1101,12 +1101,12 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
// Note that there may be multiple predecessor blocks, so we cannot move
// bonus instructions to a predecessor block.
for (Instruction &BonusInst : *BB) {
- if (isa<DbgInfoIntrinsic>(BonusInst) || BonusInst.isTerminator())
+ if (BonusInst.isTerminator())
continue;
Instruction *NewBonusInst = BonusInst.clone();
- if (PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
+ if (!isa<DbgInfoIntrinsic>(BonusInst) && PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
// Unless the instruction has the same !dbg location as the original
// branch, drop it. When we fold the bonus instructions we want to make
// sure we reset their debug locations in order to avoid stepping on
@@ -1126,6 +1126,10 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
NewBonusInst->dropUBImplyingAttrsAndMetadata();
NewBonusInst->insertInto(PredBlock, PTI->getIterator());
+
+ if (isa<DbgInfoIntrinsic>(BonusInst))
+ continue;
+
NewBonusInst->takeName(&BonusInst);
BonusInst.setName(NewBonusInst->getName() + ".old");
@@ -3744,16 +3748,6 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
PBI->setCondition(
createLogicalOp(Builder, Opc, PBI->getCondition(), BICond, "or.cond"));
- // Copy any debug value intrinsics into the end of PredBlock.
- for (Instruction &I : *BB) {
- if (isa<DbgInfoIntrinsic>(I)) {
- Instruction *NewI = I.clone();
- RemapInstruction(NewI, VMap,
- RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
- NewI->insertBefore(PBI);
- }
- }
-
++NumFoldBranchToCommonDest;
return true;
}
diff --git a/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll b/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
index 41486c5935a392c..e8b8cc6edafe0c2 100644
--- a/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
+++ b/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
@@ -8,23 +8,26 @@
define i1 @foo(i32) nounwind ssp !dbg !0 {
; CHECK-LABEL: @foo(
; CHECK-NEXT: Entry:
-; CHECK-NEXT: [[TMP1:%.*]] = icmp slt i32 [[TMP0:%.*]], 0
-; CHECK-NEXT: [[TMP2:%.*]] = icmp sgt i32 [[TMP0]], 4
-; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[TMP1]], [[TMP2]]
-; CHECK-NEXT: br i1 [[OR_COND]], label [[COMMON_RET:%.*]], label [[BB2:%.*]]
+; CHECK-NEXT: [[TMP1:%.*]] = icmp slt i32 [[TMP0:%.*]], 0, !dbg [[DBG7:![0-9]+]]
+; CHECK-NEXT: [[TMP2:%.*]] = icmp sgt i32 [[TMP0]], 4, !dbg [[DBG7]]
+; CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[TMP1]], [[TMP2]], !dbg [[DBG7]]
+; CHECK-NEXT: br i1 [[OR_COND]], label [[COMMON_RET:%.*]], label [[BB2:%.*]], !dbg [[DBG7]]
; CHECK: BB2:
-; CHECK-NEXT: [[TMP3:%.*]] = shl i32 1, [[TMP0]]
-; CHECK-NEXT: [[TMP4:%.*]] = and i32 [[TMP3]], 31
-; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i32 [[TMP4]], 0
+; CHECK-NEXT: [[TMP3:%.*]] = shl i32 1, [[TMP0]], !dbg [[DBG7]]
+; CHECK-NEXT: [[TMP4:%.*]] = and i32 [[TMP3]], 31, !dbg [[DBG7]]
+; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i32 [[TMP4]], 0, !dbg [[DBG7]]
+; CHECK-NEXT: call void @llvm.dbg.value(metadata ptr null, metadata [[META8:![0-9]+]], metadata !DIExpression()), !dbg [[DBG13:![0-9]+]]
; CHECK-NEXT: [[TMP6:%.*]] = getelementptr inbounds [5 x %0], ptr @[[GLOB0:[0-9]+]], i32 0, i32 [[TMP0]]
+; CHECK-NEXT: call void @llvm.dbg.value(metadata ptr [[TMP6]], metadata [[META8]], metadata !DIExpression()), !dbg [[DBG13]]
; CHECK-NEXT: [[TMP7:%.*]] = icmp eq ptr [[TMP6]], null
-; CHECK-NEXT: [[OR_COND2:%.*]] = select i1 [[TMP5]], i1 true, i1 [[TMP7]]
+; CHECK-NEXT: call void @llvm.dbg.value(metadata ptr [[TMP6]], metadata [[META8]], metadata !DIExpression()), !dbg [[DBG13]]
+; CHECK-NEXT: [[OR_COND2:%.*]] = select i1 [[TMP5]], i1 true, i1 [[TMP7]], !dbg [[DBG7]]
; CHECK-NEXT: [[TMP8:%.*]] = icmp slt i32 [[TMP0]], 0
-; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[OR_COND2]], i1 false, i1 [[TMP8]]
-; CHECK-NEXT: br label [[COMMON_RET]]
+; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[OR_COND2]], i1 false, i1 [[TMP8]], !dbg [[DBG7]]
+; CHECK-NEXT: br label [[COMMON_RET]], !dbg [[DBG7]]
; CHECK: common.ret:
; CHECK-NEXT: [[COMMON_RET_OP:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[SPEC_SELECT]], [[BB2]] ]
-; CHECK-NEXT: ret i1 [[COMMON_RET_OP]]
+; CHECK-NEXT: ret i1 [[COMMON_RET_OP]], !dbg [[DBG14:![0-9]+]]
;
Entry:
%1 = icmp slt i32 %0, 0, !dbg !5
@@ -42,9 +45,11 @@ BB2: ; preds = %BB1
BB3: ; preds = %BB2
+ call void @llvm.dbg.value(metadata ptr null, metadata !7, metadata !DIExpression()), !dbg !12
%6 = getelementptr inbounds [5 x %0], ptr @0, i32 0, i32 %0, !dbg !6
- call void @llvm.dbg.value(metadata ptr %6, metadata !7, metadata !{}), !dbg !12
+ call void @llvm.dbg.value(metadata ptr %6, metadata !7, metadata !DIExpression()), !dbg !12
%7 = icmp eq ptr %6, null, !dbg !13
+ call void @llvm.dbg.value(metadata ptr %6, metadata !7, metadata !DIExpression()), !dbg !12
br i1 %7, label %BB5, label %BB4, !dbg !13
BB4: ; preds = %BB3
@@ -58,12 +63,13 @@ BB5: ; preds = %BB3, %BB2, %BB1, %E
declare void @llvm.dbg.value(metadata, metadata, metadata) nounwind readnone
!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!16, !17}
!0 = distinct !DISubprogram(name: "foo", line: 231, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !2, file: !15, scope: !1, type: !3)
!1 = !DIFile(filename: "a.c", directory: "/private/tmp")
!2 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang (trunk 129006)", isOptimized: true, emissionKind: FullDebug, file: !15, enums: !4, retainedTypes: !4)
!3 = !DISubroutineType(types: !4)
-!4 = !{null}
+!4 = !{}
!5 = !DILocation(line: 131, column: 2, scope: !0)
!6 = !DILocation(line: 134, column: 2, scope: !0)
!7 = !DILocalVariable(name: "bar", line: 232, scope: !8, file: !1, type: !9)
@@ -75,3 +81,5 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) nounwind readnone
!13 = !DILocation(line: 234, column: 2, scope: !8)
!14 = !DILocation(line: 274, column: 1, scope: !8)
!15 = !DIFile(filename: "a.c", directory: "/private/tmp")
+!16 = !{i32 1, !"Debug Info Version", i32 3}
+!17 = !{i32 2, !"Dwarf Version", i32 4}
>From 4b749c4e54e4c9d5d464675924d9226b546bad5e Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 16 Nov 2023 17:33:34 +0000
Subject: [PATCH 2/3] Don't add dbg.values to the remap-map (it's wasteful),
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 401a71d59f3324e..2c1aa49b9c87092 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1116,7 +1116,6 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
RemapInstruction(NewBonusInst, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
- VMap[&BonusInst] = NewBonusInst;
// If we speculated an instruction, we need to drop any metadata that may
// result in undefined behavior, as the metadata might have been valid
@@ -1132,6 +1131,7 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
NewBonusInst->takeName(&BonusInst);
BonusInst.setName(NewBonusInst->getName() + ".old");
+ VMap[&BonusInst] = NewBonusInst;
// Update (liveout) uses of bonus instructions,
// now that the bonus instruction has been cloned into predecessor.
>From 5246cf63b965deb58d939478183d0b0d9bd5cf5f Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 16 Nov 2023 17:34:11 +0000
Subject: [PATCH 3/3] clang-format
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 2c1aa49b9c87092..9f4f0f25129047b 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1106,7 +1106,8 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
Instruction *NewBonusInst = BonusInst.clone();
- if (!isa<DbgInfoIntrinsic>(BonusInst) && PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
+ if (!isa<DbgInfoIntrinsic>(BonusInst) &&
+ PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
// Unless the instruction has the same !dbg location as the original
// branch, drop it. When we fold the bonus instructions we want to make
// sure we reset their debug locations in order to avoid stepping on
More information about the llvm-commits
mailing list