From 51d4c7ceea61b5d83ba52398b5ca6d58d7551044 Mon Sep 17 00:00:00 2001 From: Mikael Holmen Date: Fri, 2 Sep 2022 10:56:11 +0200 Subject: [PATCH] [GlobalOpt] Fix debug variance problem in hasOnlyColdCalls hasOnlyColdCalls skipped over calls to intrinsics, but it did so after checking the linkage of the called function. This meant that the presence of a call to a debug intrinsic could affect the outcome of the optimization. In my original reproducer (for an out of tree target) it was particularly interesting, because the actual IR after GlobalOpt was not different with debug instrinsics present, so -print-after-all printouts didn't show anything there. However, without debuginfo, GlobalOpt went further and ran BlockFrequencyAnalysis and (more importanly) LoopAnalysis, and later on in the pipeline, instcombine behaved in different ways when LoopInfo was present. So a call to a dbg.declare prevented running LoopAnalysis in GlobalOpt, which later prevented InstCombine from doing an optimization. The dbg-intrinsic-loopanalysis.ll testcase tries to expose this. Then I also noted that adding a dbg.declare actually made the existing testcase colccc_coldsites.ll generate different code, so I modified that to now test it behaves the same way with and without the dbg.declare. Reviewed By: nikic, fhahn Differential Revision: https://reviews.llvm.org/D133193 --- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 7 +++- .../GlobalOpt/PowerPC/coldcc_coldsites.ll | 26 +++++++++++- .../GlobalOpt/dbg-intrinsic-loopanalysis.ll | 46 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 llvm/test/Transforms/GlobalOpt/dbg-intrinsic-loopanalysis.ll diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index ff096b4..094ac81 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -1821,11 +1821,14 @@ hasOnlyColdCalls(Function &F, Function *CalledFn = CI->getCalledFunction(); if (!CalledFn) return false; - if (!CalledFn->hasLocalLinkage()) - return false; // Skip over intrinsics since they won't remain as function calls. + // Important to do this check before the linkage check below so we + // won't bail out on debug intrinsics, possibly making the generated + // code dependent on the presence of debug info. if (CalledFn->getIntrinsicID() != Intrinsic::not_intrinsic) continue; + if (!CalledFn->hasLocalLinkage()) + return false; // Check if it's valid to use coldcc calling convention. if (!hasChangeableCC(CalledFn) || CalledFn->isVarArg() || CalledFn->hasAddressTaken()) diff --git a/llvm/test/Transforms/GlobalOpt/PowerPC/coldcc_coldsites.ll b/llvm/test/Transforms/GlobalOpt/PowerPC/coldcc_coldsites.ll index 2aba30b..38628d5 100644 --- a/llvm/test/Transforms/GlobalOpt/PowerPC/coldcc_coldsites.ll +++ b/llvm/test/Transforms/GlobalOpt/PowerPC/coldcc_coldsites.ll @@ -1,7 +1,9 @@ +; RUN: opt -strip-debug -S < %s | opt -passes=globalopt -mtriple=powerpc64le-unknown-linux-gnu -ppc-enable-coldcc -S | FileCheck %s -check-prefix=COLDCC +; RUN: opt -strip-debug -S < %s | opt -passes=globalopt -S | FileCheck %s -check-prefix=CHECK ; RUN: opt -passes=globalopt -mtriple=powerpc64le-unknown-linux-gnu -ppc-enable-coldcc -S < %s | FileCheck %s -check-prefix=COLDCC ; RUN: opt -passes=globalopt -S < %s | FileCheck %s -check-prefix=CHECK -define signext i32 @caller(i32 signext %a, i32 signext %b, i32 signext %lim, i32 signext %i) local_unnamed_addr #0 !prof !30 { +define signext i32 @caller(i32 signext %a, i32 signext %b, i32 signext %lim, i32 signext %i, ptr %k) local_unnamed_addr #0 !prof !30 { entry: ; COLDCC: call coldcc signext i32 @callee ; CHECK: call fastcc signext i32 @callee @@ -11,6 +13,7 @@ entry: br i1 %cmp, label %if.then, label %if.end, !prof !31 if.then: ; preds = %entry + call void @llvm.dbg.declare(metadata ptr %k, metadata !101, metadata !DIExpression()), !dbg !122 %call = tail call signext i32 @callee(i32 signext %a, i32 signext %b) br label %if.end @@ -45,7 +48,15 @@ for.body: ; preds = %for.body, %entry %exitcond = icmp eq i32 %inc, 10000000 br i1 %exitcond, label %for.cond.cleanup, label %for.body, !prof !34 } + +; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn +declare void @llvm.dbg.declare(metadata, metadata, metadata) #2 + attributes #0 = { noinline } +attributes #2 = { nocallback nofree nosync nounwind readnone speculatable willreturn } + +!llvm.dbg.cu = !{!117} +!llvm.module.flags = !{!100} !0 = !{i32 1, !"ProfileSummary", !1} !1 = !{!2, !3, !4, !5, !6, !7, !8, !9} @@ -79,3 +90,16 @@ attributes #0 = { noinline } !32 = !{i32 59} !33 = !{!"function_entry_count", i64 1} !34 = !{!"branch_weights", i32 2, i32 10000001} + +!100 = !{i32 2, !"Debug Info Version", i32 3} +!101 = !DILocalVariable(name: "k", arg: 1, scope: !102, file: !103, line: 13, type: !119) +!102 = distinct !DISubprogram(name: "h", scope: !103, file: !103, line: 13, type: !104, scopeLine: 13, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !117, retainedNodes: !121) +!103 = !DIFile(filename: "foo2.c", directory: "/bar") +!104 = !DISubroutineType(types: !105) +!105 = !{!119} +!117 = distinct !DICompileUnit(language: DW_LANG_C99, file: !103, producer: "clang version 16", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !118, globals: !120, splitDebugInlining: false, nameTableKind: None) +!118 = !{!119} +!119 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed) +!120 = !{} +!121 = !{!101} +!122 = !DILocation(line: 13, column: 27, scope: !102) diff --git a/llvm/test/Transforms/GlobalOpt/dbg-intrinsic-loopanalysis.ll b/llvm/test/Transforms/GlobalOpt/dbg-intrinsic-loopanalysis.ll new file mode 100644 index 0000000..2e82fe5 --- /dev/null +++ b/llvm/test/Transforms/GlobalOpt/dbg-intrinsic-loopanalysis.ll @@ -0,0 +1,46 @@ +; RUN: opt -passes="globalopt" < %s -o /dev/null -debug-pass-manager 2>&1 | FileCheck %s +; RUN: opt -strip-debug -S < %s | opt -passes="globalopt" -o /dev/null -debug-pass-manager 2>&1 | FileCheck %s + +; Make sure that the call to dbg.declare does not prevent running BlockFrequency +; and (especially) Loop Analysis. +; Later passes (e.g. instcombine) may behave in different ways depending on if +; LoopInfo is available or not. Therefore, letting GlobalOpt run or not run +; LoopAnalysis depending on the presence of a dbg.declare may make the compiler +; generate different code with and without debug info. + +; CHECK: Running pass: GlobalOptPass on [module] +; CHECK: Running analysis: BlockFrequencyAnalysis on h +; CHECK: Running analysis: LoopAnalysis on h + +define i16 @h(ptr %k) { +entry: + call void @llvm.dbg.declare(metadata ptr %k, metadata !1, metadata !DIExpression()), !dbg !22 + %call = call i16 @gaz() + ret i16 %call +} + +; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn +declare void @llvm.dbg.declare(metadata, metadata, metadata) #0 + +define internal i16 @gaz() { +entry: + ret i16 0 +} + +attributes #0 = { nocallback nofree nosync nounwind readnone speculatable willreturn } + +!llvm.dbg.cu = !{!17} +!llvm.module.flags = !{!0} + +!0 = !{i32 2, !"Debug Info Version", i32 3} +!1 = !DILocalVariable(name: "k", arg: 1, scope: !2, file: !3, line: 13, type: !19) +!2 = distinct !DISubprogram(name: "h", scope: !3, file: !3, line: 13, type: !4, scopeLine: 13, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !17, retainedNodes: !21) +!3 = !DIFile(filename: "foo2.c", directory: "/bar") +!4 = !DISubroutineType(types: !5) +!5 = !{!19} +!17 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 16", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !18, globals: !20, splitDebugInlining: false, nameTableKind: None) +!18 = !{!19} +!19 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed) +!20 = !{} +!21 = !{!1} +!22 = !DILocation(line: 13, column: 27, scope: !2) -- 2.7.4