Skip to content

[DebugInfo][ConstraintElimination] Fix debug value loss in replacing comparisons with the speculated constants #136839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 5, 2025
Merged

[DebugInfo][ConstraintElimination] Fix debug value loss in replacing comparisons with the speculated constants #136839

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af4944f
Fix #135736: salvage debug values with replaced constants
Apochens Apr 23, 2025
4625651
fix clang-tidy
Apochens Apr 23, 2025
aab3828
salvaging debug values outside the if condition and refine tests
Apochens Apr 24, 2025
598197b
test nits
Apochens Apr 30, 2025
3a04fe1
nit
Apochens Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/InstrTypes.h"
Expand Down Expand Up @@ -1454,8 +1455,29 @@ static bool checkAndReplaceCondition(
return ShouldReplace;
});
NumCondsRemoved++;

// Update the debug value records that satisfy the same condition used
// in replaceUsesWithIf.
SmallVector<DbgVariableIntrinsic *> DbgUsers;
SmallVector<DbgVariableRecord *> DVRUsers;
findDbgUsers(DbgUsers, Cmp, &DVRUsers);

for (auto *DVR : DVRUsers) {
auto *DTN = DT.getNode(DVR->getParent());
if (!DTN || DTN->getDFSNumIn() < NumIn || DTN->getDFSNumOut() > NumOut)
continue;

auto *MarkedI = DVR->getInstruction();
if (MarkedI->getParent() == ContextInst->getParent() &&
MarkedI->comesBefore(ContextInst))
continue;
Comment on lines +1465 to +1473
Copy link
Contributor

@OCHyams OCHyams Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, part of me wants to reduce code duplication by introducing a lambda that is used by this code and the checks in replaceUsesWithIf above. That gives an added benefit of naming what it's doing. On the other hand, that might be more boilerplate than is needed here, so I'm happy for it to stay as it is too.


DVR->replaceVariableLocationOp(Cmp, ConstantC);
}

if (Cmp->use_empty())
ToRemove.push_back(Cmp);

return Changed;
};

Expand Down
83 changes: 83 additions & 0 deletions llvm/test/Transforms/ConstraintElimination/salvage-dbg-values-replaced-by-constant-2.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
; RUN: opt < %s -passes=constraint-elimination -S | FileCheck %s

; Check that checkAndReplaceCondition() salvages the debug value information after replacing
; the conditions (`%c.1` and `%t.2` in this test) with the speculated constants (GitHub Issue
; #135736).
; In particular, the debug value record uses should not be replaced if they comes before the
; context instrtuction (e.g., `%t.2` in this example).
Copy link
Contributor

@OCHyams OCHyams Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: comes -> come


declare void @llvm.assume(i1 noundef) #0

declare void @may_unwind()

declare void @use(i1)

define i1 @assume_single_bb_conditions_after_assume(i8 %a, i8 %b, i1 %c) !dbg !5 {
; CHECK-LABEL: define i1 @assume_single_bb_conditions_after_assume(
; CHECK: [[CMP_1:%.*]] = icmp ule i8 [[ADD_1:%.*]], [[B:%.*]], !dbg [[DBG12:![0-9]+]]
; CHECK-NEXT: [[C_1:%.*]] = icmp ule i8 [[ADD_1]], [[B]], !dbg [[DBG13:![0-9]+]]
; CHECK-NEXT: #dbg_value(i1 [[C_1]], [[META9:![0-9]+]], !DIExpression(), [[META14:![0-9]+]])
; CHECK-NEXT: call void @use(i1 [[C_1]]), !dbg [[DBG15:![0-9]+]]
; CHECK-NEXT: #dbg_value(i1 [[C_1]], [[META9]], !DIExpression(), [[META14]])
; CHECK-NEXT: call void @may_unwind(), !dbg [[DBG16:![0-9]+]]
; CHECK-NEXT: #dbg_value(i1 [[C_1]], [[META9]], !DIExpression(), [[META14]])
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP_1]]), !dbg [[DBG17:![0-9]+]]
; CHECK-NEXT: #dbg_value(i1 [[C_1]], [[META9]], !DIExpression(), [[META14]])
; CHECK-NEXT: #dbg_value(i1 true, [[META9]], !DIExpression(), [[META14]])
; CHECK-NEXT: [[RES_1:%.*]] = xor i1 true, true, !dbg [[DBG18:![0-9]+]]
;
%add.1 = add nuw nsw i8 %a, 1, !dbg !11
%cmp.1 = icmp ule i8 %add.1, %b, !dbg !12
%c.1 = icmp ule i8 %add.1, %b, !dbg !13
#dbg_value(i1 %c.1, !9, !DIExpression(), !14)
call void @use(i1 %c.1), !dbg !15
#dbg_value(i1 %c.1, !9, !DIExpression(), !14)
call void @may_unwind(), !dbg !16
#dbg_value(i1 %c.1, !9, !DIExpression(), !14)
call void @llvm.assume(i1 %cmp.1), !dbg !17
#dbg_value(i1 %c.1, !9, !DIExpression(), !14)
%t.2 = icmp ule i8 %a, %b, !dbg !14
#dbg_value(i1 %c.1, !9, !DIExpression(), !14)
%res.1 = xor i1 %c.1, %t.2, !dbg !18
%add.2 = add nuw nsw i8 %a, 2, !dbg !19
%c.2 = icmp ule i8 %add.2, %b, !dbg !20
%res.2 = xor i1 %res.1, %c.2, !dbg !21
ret i1 %res.2, !dbg !22
}

attributes #0 = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) }

!llvm.dbg.cu = !{!0}
!llvm.debugify = !{!2, !3}
!llvm.module.flags = !{!4}

!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
!1 = !DIFile(filename: "/app/example.ll", directory: "/")
!2 = !{i32 12}
!3 = !{i32 8}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = distinct !DISubprogram(name: "assume_single_bb_conditions_after_assume", linkageName: "assume_single_bb_conditions_after_assume", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
!6 = !DISubroutineType(types: !7)
!7 = !{}
!8 = !{!9}
!9 = !DILocalVariable(name: "4", scope: !5, file: !1, line: 7, type: !10)
!10 = !DIBasicType(name: "ty8", size: 8, encoding: DW_ATE_unsigned)
!11 = !DILocation(line: 1, column: 1, scope: !5)
!12 = !DILocation(line: 2, column: 1, scope: !5)
!13 = !DILocation(line: 3, column: 1, scope: !5)
!14 = !DILocation(line: 7, column: 1, scope: !5)
!15 = !DILocation(line: 4, column: 1, scope: !5)
!16 = !DILocation(line: 5, column: 1, scope: !5)
!17 = !DILocation(line: 6, column: 1, scope: !5)
!18 = !DILocation(line: 8, column: 1, scope: !5)
!19 = !DILocation(line: 9, column: 1, scope: !5)
!20 = !DILocation(line: 10, column: 1, scope: !5)
!21 = !DILocation(line: 11, column: 1, scope: !5)
!22 = !DILocation(line: 12, column: 1, scope: !5)

; CHECK: [[META9]] = !DILocalVariable(name: "4",
; CHECK: [[DBG12]] = !DILocation(line: 2, column: 1,
; CHECK: [[DBG13]] = !DILocation(line: 3, column: 1,
; CHECK: [[META14]] = !DILocation(line: 7, column: 1,
; CHECK: [[DBG18]] = !DILocation(line: 8, column: 1,
;.
65 changes: 65 additions & 0 deletions llvm/test/Transforms/ConstraintElimination/salvage-dbg-values-replaced-by-constant.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
; RUN: opt < %s -passes=constraint-elimination -S | FileCheck %s

; Check that checkAndReplaceCondition() salvages the debug value information after replacing
; the conditions (`%t.1` in this test) with the speculated constants (GitHub Issue #135736).
; In particular, the debug value record uses are replaced if their `DFSNumIn`s and `DFSNumOut`s
; on `DominatorTree` satisfy the contraint.
Copy link
Contributor

@OCHyams OCHyams Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment feels slightly awkward to parse - perhaps something like "In particular, debug uses are replaced if the debug record is dominated by the condition fact" (@fhahn might be able to produce better wording?)


define i1 @test_and_ule(i4 %x, i4 %y, i4 %z) !dbg !5 {
; CHECK-LABEL: define i1 @test_and_ule(
; CHECK-SAME: i4 [[X:%.*]], i4 [[Y:%.*]], i4 [[Z:%.*]])
; CHECK: [[T_1:%.*]] = icmp ule i4 [[X]], [[Z]], !dbg [[DBG13:![0-9]+]]
;
entry:
%c.1 = icmp ule i4 %x, %y, !dbg !11
%c.2 = icmp ule i4 %y, %z, !dbg !12
%t.1 = icmp ule i4 %x, %z, !dbg !13
%and = and i1 %c.1, %c.2, !dbg !14
br i1 %and, label %then, label %exit, !dbg !15

; CHECK: [[THEN:.*]]:
; CHECK-NEXT: #dbg_value(i1 true, [[META9:![0-9]+]], !DIExpression(), [[DBG13]])
; CHECK-NEXT: [[R_1:%.*]] = xor i1 true, true, !dbg [[DBG16:![0-9]+]]
then: ; preds = %entry
#dbg_value(i1 %t.1, !9, !DIExpression(), !13)
%r.1 = xor i1 %t.1, %t.1, !dbg !16
br label %exit

; CHECK: [[EXIT:.*]]:
; CHECK-NEXT: #dbg_value(i1 [[T_1]], [[META17:![0-9]+]], !DIExpression(), [[DBG13]])
; CHECK-NEXT: ret i1 [[T_1]], !dbg [[DBG18:![0-9]+]]
exit: ; preds = %bb1, %entry
#dbg_value(i1 %t.1, !17, !DIExpression(), !13)
ret i1 %t.1, !dbg !18
}

!llvm.dbg.cu = !{!0}
!llvm.debugify = !{!2, !3}
!llvm.module.flags = !{!4}

!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
!1 = !DIFile(filename: "temp.ll", directory: "/")
!2 = !{i32 20}
!3 = !{i32 17}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = distinct !DISubprogram(name: "test_and_ule", linkageName: "test_and_ule", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
!6 = !DISubroutineType(types: !7)
!7 = !{}
!8 = !{!9}
!9 = !DILocalVariable(name: "4", scope: !5, file: !1, line: 5, type: !10)
!10 = !DIBasicType(name: "ty8", size: 8, encoding: DW_ATE_unsigned)
!11 = !DILocation(line: 1, column: 1, scope: !5)
!12 = !DILocation(line: 2, column: 1, scope: !5)
!13 = !DILocation(line: 5, column: 1, scope: !5)
!14 = !DILocation(line: 3, column: 1, scope: !5)
!15 = !DILocation(line: 4, column: 1, scope: !5)
!16 = !DILocation(line: 7, column: 1, scope: !5)
!17 = !DILocalVariable(name: "6", scope: !5, file: !1, line: 7, type: !10)
!18 = !DILocation(line: 20, column: 1, scope: !5)

; CHECK: [[META9]] = !DILocalVariable(name: "4",
; CHECK: [[DBG13]] = !DILocation(line: 5, column: 1,
; CHECK: [[DBG16]] = !DILocation(line: 7, column: 1,
; CHECK: [[META17]] = !DILocalVariable(name: "6",
; CHECK: [[DBG18]] = !DILocation(line: 20, column: 1,
;.
Loading