Skip to content

Commit

Permalink
[ADCE] Only remove debug intrinsics if non debug instructions are rem…
Browse files Browse the repository at this point in the history
…oved

We now limit ADCE to only remove debug intrinsics if it does something else
that would invalidate cached analyses anyway.
As we've seen in
 #58285
throwing away cached analysis info when only debug instructions are removed
can lead to different code when debug info is present or not present.

Differential Revision: https://reviews.llvm.org/D145051
  • Loading branch information
mikaelholmen committed Mar 3, 2023
1 parent 5783363 commit 8aa9ab3
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 7 deletions.
26 changes: 19 additions & 7 deletions llvm/lib/Transforms/Scalar/ADCE.cpp
Expand Up @@ -540,6 +540,8 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() {
}
});

bool FoundNonDebugInstr = false;

// The inverse of the live set is the dead set. These are those instructions
// that have no side effects and do not influence the control flow or return
// value of the function, and may therefore be deleted safely.
Expand All @@ -560,22 +562,32 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() {
continue;

// Fallthrough and drop the intrinsic.
} else {
// Remember that we found some non-debug instructions to delete.
FoundNonDebugInstr = true;
}

// Prepare to delete.
Worklist.push_back(&I);
salvageDebugInfo(I);
}

for (Instruction *&I : Worklist)
I->dropAllReferences();
Changed.ChangedAnything = Changed.ChangedControlFlow || FoundNonDebugInstr;

for (Instruction *&I : Worklist) {
++NumRemoved;
I->eraseFromParent();
}
// Only actually remove the dead instructions if we know we will invalidate
// cached analysis results. If we only found debug intrinsics to remove and
// we then invalidate analyses because of that, later passes may behave
// differently which then makes the presence of debug info affect code
// generation.
if (Changed.ChangedAnything) {
for (Instruction *&I : Worklist)
I->dropAllReferences();

Changed.ChangedAnything = Changed.ChangedControlFlow || !Worklist.empty();
for (Instruction *&I : Worklist) {
++NumRemoved;
I->eraseFromParent();
}
}

return Changed;
}
Expand Down
56 changes: 56 additions & 0 deletions llvm/test/Transforms/ADCE/debug-info-intrinsic-2.ll
@@ -0,0 +1,56 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=adce -S < %s | FileCheck %s

; ADCE can remove the dbg.declare in remove_dbg_declare_and_other as it is also
; removing a non-debug instruction.

define i16 @remove_dbg_declare_and_other() {
; CHECK-LABEL: @remove_dbg_declare_and_other(
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i16 0
;
entry:
call void @llvm.dbg.declare(metadata ptr undef, metadata !4, metadata !DIExpression()), !dbg !16
%dead = add i8 0, 0
ret i16 0
}

; In dont_remove_only_dbg_declare we must leave the dbg.declare since we're not
; removing any non-debug instructions.

define i16 @dont_remove_only_dbg_declare() {
; CHECK-LABEL: @dont_remove_only_dbg_declare(
; CHECK-NEXT: entry:
; CHECK-NEXT: call void @llvm.dbg.declare(metadata ptr undef, metadata [[META4:![0-9]+]], metadata !DIExpression()), !dbg [[DBG16:![0-9]+]]
; CHECK-NEXT: ret i16 0
;
entry:
call void @llvm.dbg.declare(metadata ptr undef, metadata !4, metadata !DIExpression()), !dbg !16
ret i16 0
}

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare void @llvm.dbg.declare(metadata, metadata, metadata) #0

attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

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

!0 = !{i32 7, !"Dwarf Version", i32 4}
!1 = !{i32 2, !"Debug Info Version", i32 3}
!2 = !{i32 1, !"wchar_size", i32 1}
!3 = !{i32 7, !"frame-pointer", i32 2}
!4 = !DILocalVariable(name: "w", scope: !5, file: !6, line: 18, type: !11)
!5 = distinct !DILexicalBlock(scope: !7, file: !6, line: 18, column: 8)
!6 = !DIFile(filename: "foo2.c", directory: "/llvm")
!7 = distinct !DISubprogram(name: "test1", scope: !6, file: !6, line: 14, type: !8, scopeLine: 14, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !10)
!8 = !DISubroutineType(types: !9)
!9 = !{}
!10 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: "clang version 16.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !9, splitDebugInlining: false, nameTableKind: None)
!11 = !DIDerivedType(tag: DW_TAG_typedef, name: "uint64_t", file: !12, line: 60, baseType: !13)
!12 = !DIFile(filename: "/include/sys/_stdint.h", directory: "")
!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "__uint64_t", file: !14, line: 108, baseType: !15)
!14 = !DIFile(filename: "/include/machine/_default_types.h", directory: "")
!15 = !DIBasicType(name: "unsigned long long", size: 64, encoding: DW_ATE_unsigned)
!16 = !DILocation(line: 18, column: 8, scope: !5)
10 changes: 10 additions & 0 deletions llvm/test/Transforms/ADCE/debug-info-intrinsic.ll
Expand Up @@ -26,6 +26,14 @@
;; sink();
;;}

; Note: The instruction
; %dead = add i8 0, 0
; in
; @variable_in_unused_subscope
; and
; @calls_empty_function_with_unused_variable_in_unused_subscope
; is just there to let ADCE remove debug intrinsics at all. It only removes
; debug intrinsics if it also found something else to remove.
declare void @llvm.dbg.value(metadata, metadata, metadata)

declare void @sink()
Expand All @@ -38,6 +46,7 @@ define void @variable_in_unused_subscope() !dbg !4 {
entry:
call void @llvm.dbg.value(metadata i32 0, metadata !15, metadata !17), !dbg !18
call void @sink(), !dbg !19
%dead = add i8 0, 0
ret void, !dbg !20
}

Expand All @@ -61,6 +70,7 @@ define void @calls_empty_function_with_unused_variable_in_unused_subscope() !dbg
entry:
call void @llvm.dbg.value(metadata i32 0, metadata !26, metadata !17), !dbg !28
call void @sink(), !dbg !31
%dead = add i8 0, 0
ret void, !dbg !32
}

Expand Down
10 changes: 10 additions & 0 deletions llvm/test/Transforms/MergeFunc/mergefunc-preserve-debug-info.ll
Expand Up @@ -147,6 +147,16 @@ entry:

; OPTIMIZATION_LEVEL_2: define void @f()
; OPTIMIZATION_LEVEL_2-NEXT: entry:
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.value(metadata i32 3, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.value(metadata i32 4, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.declare(metadata ptr undef, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.declare(metadata ptr undef, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.value(metadata i32 4, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.value(metadata i32 1, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.value(metadata i32 9, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.declare(metadata ptr undef, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.declare(metadata ptr undef, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: call void @llvm.dbg.value(metadata i32 9, metadata !{{[0-9]+}}, metadata !DIExpression()), !dbg !{{[0-9]+}}
; OPTIMIZATION_LEVEL_2-NEXT: ret void, !dbg !{{[0-9]+}}

%call = call i32 @maxA(i32 3, i32 4), !dbg !60
Expand Down

0 comments on commit 8aa9ab3

Please sign in to comment.