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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "llvm/Analysis/ScalarEvolutionExpressions.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.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

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
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 come before the
; context instrtuction (e.g., `%t.2` in this example).

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,
;.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
; 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, debug uses are replaced if the debug record is dominated by the condition fact.

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,
;.