From edd6c8b874453de438ea787d63d4ffb9ab7b43a4 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 12 Apr 2021 14:51:51 -0400 Subject: [PATCH] [PassManager][PhaseOrdering] lower expects before running simplifyCFG Retry of 330619a3a623 that includes a clang test update. Original commit message: If we run passes before lowering llvm.expect intrinsics to metadata, then those passes have no way to act on the hints provided by llvm.expect. SimplifyCFG is the known offender, and we made it smarter about profile metadata in D98898 . In the motivating example from https://llvm.org/PR49336 , this means we were ignoring the recommended method for a programmer to tell the compiler that a compare+branch is expensive. This change appears to solve that case - the metadata survives to the backend, the compare order is as expected in IR, and the backend does not do anything to reverse it. We make the same change to the old pass manager to keep things synchronized. Differential Revision: https://reviews.llvm.org/D100213 --- lib/Passes/PassBuilder.cpp | 4 +++- lib/Transforms/IPO/PassManagerBuilder.cpp | 4 +++- test/CodeGen/AMDGPU/opt-pipeline.ll | 6 +++--- test/Other/new-pm-defaults.ll | 2 +- test/Other/new-pm-pgo.ll | 2 +- test/Other/new-pm-thinlto-defaults.ll | 2 +- test/Other/new-pm-thinlto-postlink-pgo-defaults.ll | 2 +- .../new-pm-thinlto-postlink-samplepgo-defaults.ll | 2 +- test/Other/new-pm-thinlto-prelink-pgo-defaults.ll | 2 +- .../Other/new-pm-thinlto-prelink-samplepgo-defaults.ll | 2 +- test/Other/opt-O2-pipeline.ll | 2 +- test/Other/opt-O3-pipeline-enable-matrix.ll | 2 +- test/Other/opt-O3-pipeline.ll | 2 +- test/Other/opt-Os-pipeline.ll | 2 +- test/Transforms/PhaseOrdering/expect.ll | 10 +++++----- 15 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/Passes/PassBuilder.cpp b/lib/Passes/PassBuilder.cpp index 6307e468e70..75ba9da4214 100644 --- a/lib/Passes/PassBuilder.cpp +++ b/lib/Passes/PassBuilder.cpp @@ -1066,10 +1066,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, // Create an early function pass manager to cleanup the output of the // frontend. FunctionPassManager EarlyFPM(DebugLogging); + // Lower llvm.expect to metadata before attempting transforms. + // Compare/branch metadata may alter the behavior of passes like SimplifyCFG. + EarlyFPM.addPass(LowerExpectIntrinsicPass()); EarlyFPM.addPass(SimplifyCFGPass()); EarlyFPM.addPass(SROA()); EarlyFPM.addPass(EarlyCSEPass()); - EarlyFPM.addPass(LowerExpectIntrinsicPass()); if (PTO.Coroutines) EarlyFPM.addPass(CoroEarlyPass()); if (Level == OptimizationLevel::O3) diff --git a/lib/Transforms/IPO/PassManagerBuilder.cpp b/lib/Transforms/IPO/PassManagerBuilder.cpp index 2c80a16febe..19e212f738a 100644 --- a/lib/Transforms/IPO/PassManagerBuilder.cpp +++ b/lib/Transforms/IPO/PassManagerBuilder.cpp @@ -316,10 +316,12 @@ void PassManagerBuilder::populateFunctionPassManager( addInitialAliasAnalysisPasses(FPM); + // Lower llvm.expect to metadata before attempting transforms. + // Compare/branch metadata may alter the behavior of passes like SimplifyCFG. + FPM.add(createLowerExpectIntrinsicPass()); FPM.add(createCFGSimplificationPass()); FPM.add(createSROAPass()); FPM.add(createEarlyCSEPass()); - FPM.add(createLowerExpectIntrinsicPass()); } // Do PGO instrumentation generation or use pass as the option specified. diff --git a/test/CodeGen/AMDGPU/opt-pipeline.ll b/test/CodeGen/AMDGPU/opt-pipeline.ll index 5a05310177b..3ba9099176d 100644 --- a/test/CodeGen/AMDGPU/opt-pipeline.ll +++ b/test/CodeGen/AMDGPU/opt-pipeline.ll @@ -41,11 +41,11 @@ ; GCN-O1-NEXT: Basic Alias Analysis (stateless AA impl) ; GCN-O1-NEXT: Function Alias Analysis Results ; GCN-O1-NEXT: Simplify well-known AMD library calls +; GCN-O1-NEXT: Lower 'expect' Intrinsics ; GCN-O1-NEXT: Simplify the CFG ; GCN-O1-NEXT: Dominator Tree Construction ; GCN-O1-NEXT: SROA ; GCN-O1-NEXT: Early CSE -; GCN-O1-NEXT: Lower 'expect' Intrinsics ; GCN-O1-NEXT: Pass Arguments: ; GCN-O1-NEXT: Target Library Information @@ -350,11 +350,11 @@ ; GCN-O2-NEXT: Basic Alias Analysis (stateless AA impl) ; GCN-O2-NEXT: Function Alias Analysis Results ; GCN-O2-NEXT: Simplify well-known AMD library calls +; GCN-O2-NEXT: Lower 'expect' Intrinsics ; GCN-O2-NEXT: Simplify the CFG ; GCN-O2-NEXT: Dominator Tree Construction ; GCN-O2-NEXT: SROA ; GCN-O2-NEXT: Early CSE -; GCN-O2-NEXT: Lower 'expect' Intrinsics ; GCN-O2-NEXT: Pass Arguments: ; GCN-O2-NEXT: Target Library Information @@ -704,11 +704,11 @@ ; GCN-O3-NEXT: Basic Alias Analysis (stateless AA impl) ; GCN-O3-NEXT: Function Alias Analysis Results ; GCN-O3-NEXT: Simplify well-known AMD library calls +; GCN-O3-NEXT: Lower 'expect' Intrinsics ; GCN-O3-NEXT: Simplify the CFG ; GCN-O3-NEXT: Dominator Tree Construction ; GCN-O3-NEXT: SROA ; GCN-O3-NEXT: Early CSE -; GCN-O3-NEXT: Lower 'expect' Intrinsics ; GCN-O3-NEXT: Pass Arguments: ; GCN-O3-NEXT: Target Library Information diff --git a/test/Other/new-pm-defaults.ll b/test/Other/new-pm-defaults.ll index 8dc54e4ccb8..ae268fd0c3a 100644 --- a/test/Other/new-pm-defaults.ll +++ b/test/Other/new-pm-defaults.ll @@ -79,6 +79,7 @@ ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis ; CHECK-O-NEXT: Running analysis: PreservedCFGCheckerAnalysis on foo ; CHECK-O-NEXT: Starting llvm::Function pass manager run. +; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running analysis: TargetIRAnalysis ; CHECK-O-NEXT: Running analysis: AssumptionAnalysis @@ -86,7 +87,6 @@ ; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis ; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass ; CHECK-O-NEXT: Finished llvm::Function pass manager run. ; CHECK-EP-PIPELINE-EARLY-SIMPLIFICATION-NEXT: Running pass: NoOpModulePass diff --git a/test/Other/new-pm-pgo.ll b/test/Other/new-pm-pgo.ll index 09fd103e172..b2f6b666150 100644 --- a/test/Other/new-pm-pgo.ll +++ b/test/Other/new-pm-pgo.ll @@ -20,10 +20,10 @@ ; USE_POST_LINK: Running pass: PGOMemOPSizeOpt ; SAMPLE_USE_O: Running pass: AddDiscriminatorsPass ; SAMPLE_USE_PRE_LINK: Running pass: AddDiscriminatorsPass +; SAMPLE_USE: Running pass: LowerExpectIntrinsicPass ; SAMPLE_USE: Running pass: SimplifyCFGPass ; SAMPLE_USE: Running pass: SROA ; SAMPLE_USE: Running pass: EarlyCSEPass -; SAMPLE_USE: Running pass: LowerExpectIntrinsicPass ; SAMPLE_USE_POST_LINK: Running pass: InstCombinePass ; SAMPLE_USE: Running pass: SampleProfileLoaderPass ; SAMPLE_USE_O: Running pass: PGOIndirectCallPromotion diff --git a/test/Other/new-pm-thinlto-defaults.ll b/test/Other/new-pm-thinlto-defaults.ll index 46620aea31d..ceb02407479 100644 --- a/test/Other/new-pm-thinlto-defaults.ll +++ b/test/Other/new-pm-thinlto-defaults.ll @@ -64,6 +64,7 @@ ; CHECK-PRELINK-O-NODIS-NEXT: Running analysis: InnerAnalysisManagerProxy ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis ; CHECK-O-NEXT: Starting llvm::Function pass manager run. +; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running analysis: TargetIRAnalysis ; CHECK-O-NEXT: Running analysis: AssumptionAnalysis @@ -71,7 +72,6 @@ ; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis ; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass ; CHECK-O-NEXT: Finished llvm::Function pass manager run. ; CHECK-POSTLINK-O-NEXT: Running pass: LowerTypeTestsPass diff --git a/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll b/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll index ed0e17175fd..77a556c278b 100644 --- a/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll +++ b/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll @@ -34,6 +34,7 @@ ; CHECK-O-NEXT: Running pass: InferFunctionAttrsPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis ; CHECK-O-NEXT: Starting {{.*}}Function pass manager run. +; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running analysis: TargetIRAnalysis ; CHECK-O-NEXT: Running analysis: AssumptionAnalysis @@ -41,7 +42,6 @@ ; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis ; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass ; CHECK-O-NEXT: Finished {{.*}}Function pass manager run. ; CHECK-O-NEXT: Running pass: LowerTypeTestsPass diff --git a/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll b/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll index 05e7a887271..b66b28f2667 100644 --- a/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll +++ b/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll @@ -36,6 +36,7 @@ ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis ; CHECK-O-NEXT: Starting {{.*}}Function pass manager run. +; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running analysis: TargetIRAnalysis ; CHECK-O-NEXT: Running analysis: AssumptionAnalysis @@ -43,7 +44,6 @@ ; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis ; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass ; CHECK-O-NEXT: Running pass: InstCombinePass on foo ; CHECK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis on foo diff --git a/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll b/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll index e1bab9c6fc7..0d7a1c44e0c 100644 --- a/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll +++ b/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll @@ -35,6 +35,7 @@ ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis ; CHECK-O-NEXT: Starting {{.*}}Function pass manager run. +; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running analysis: TargetIRAnalysis ; CHECK-O-NEXT: Running analysis: AssumptionAnalysis @@ -42,7 +43,6 @@ ; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis ; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass ; CHECK-O-NEXT: Finished {{.*}}Function pass manager run. ; CHECK-O-NEXT: Running pass: IPSCCPPass diff --git a/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll b/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll index c2412ff4073..e577e837e8e 100644 --- a/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll +++ b/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll @@ -34,6 +34,7 @@ ; CHECK-O-NEXT: Running pass: InferFunctionAttrsPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis ; CHECK-O-NEXT: Starting {{.*}}Function pass manager run. +; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O-NEXT: Running pass: SimplifyCFGPass ; CHECK-O-NEXT: Running analysis: TargetIRAnalysis ; CHECK-O-NEXT: Running analysis: AssumptionAnalysis @@ -41,7 +42,6 @@ ; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis ; CHECK-O-NEXT: Running pass: EarlyCSEPass ; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass ; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass ; CHECK-O-NEXT: Running pass: InstCombinePass on foo ; CHECK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis on foo diff --git a/test/Other/opt-O2-pipeline.ll b/test/Other/opt-O2-pipeline.ll index af7093b0bd5..36370d6cd6d 100644 --- a/test/Other/opt-O2-pipeline.ll +++ b/test/Other/opt-O2-pipeline.ll @@ -12,11 +12,11 @@ ; CHECK-NEXT: Module Verifier ; CHECK-EXT: Good Bye World Pass ; CHECK-NOEXT-NOT: Good Bye World Pass +; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Simplify the CFG ; CHECK-NEXT: Dominator Tree Construction ; CHECK-NEXT: SROA ; CHECK-NEXT: Early CSE -; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: Target Transform Information diff --git a/test/Other/opt-O3-pipeline-enable-matrix.ll b/test/Other/opt-O3-pipeline-enable-matrix.ll index 12b5e188e50..e5d9e514023 100644 --- a/test/Other/opt-O3-pipeline-enable-matrix.ll +++ b/test/Other/opt-O3-pipeline-enable-matrix.ll @@ -12,11 +12,11 @@ ; CHECK-NEXT: Module Verifier ; CHECK-EXT: Good Bye World Pass ; CHECK-NOEXT-NOT: Good Bye World Pass +; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Simplify the CFG ; CHECK-NEXT: Dominator Tree Construction ; CHECK-NEXT: SROA ; CHECK-NEXT: Early CSE -; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: Target Transform Information diff --git a/test/Other/opt-O3-pipeline.ll b/test/Other/opt-O3-pipeline.ll index bee06f816a5..af77e09c249 100644 --- a/test/Other/opt-O3-pipeline.ll +++ b/test/Other/opt-O3-pipeline.ll @@ -12,11 +12,11 @@ ; CHECK-NEXT: Module Verifier ; CHECK-EXT: Good Bye World Pass ; CHECK-NOEXT-NOT: Good Bye World Pass +; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Simplify the CFG ; CHECK-NEXT: Dominator Tree Construction ; CHECK-NEXT: SROA ; CHECK-NEXT: Early CSE -; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: Target Transform Information diff --git a/test/Other/opt-Os-pipeline.ll b/test/Other/opt-Os-pipeline.ll index 5d7ce952a78..e24d3d3f57b 100644 --- a/test/Other/opt-Os-pipeline.ll +++ b/test/Other/opt-Os-pipeline.ll @@ -12,11 +12,11 @@ ; CHECK-NEXT: Module Verifier ; CHECK-EXT: Good Bye World Pass ; CHECK-NOEXT-NOT: Good Bye World Pass +; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Simplify the CFG ; CHECK-NEXT: Dominator Tree Construction ; CHECK-NEXT: SROA ; CHECK-NEXT: Early CSE -; CHECK-NEXT: Lower 'expect' Intrinsics ; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: Target Transform Information diff --git a/test/Transforms/PhaseOrdering/expect.ll b/test/Transforms/PhaseOrdering/expect.ll index 14a9284baa5..3a23785ddff 100644 --- a/test/Transforms/PhaseOrdering/expect.ll +++ b/test/Transforms/PhaseOrdering/expect.ll @@ -1,7 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; RUN: opt -O2 -S < %s | FileCheck %s -; FIXME: ; Given an "expect" on a compare, we should not combine ; that compare with other instructions in a way that the ; backend can't undo. Expect lowering becomes metadata, @@ -10,10 +9,11 @@ define void @PR49336(i32 %delta, i32 %tag_type, i8* %ip) { ; CHECK-LABEL: @PR49336( ; CHECK-NEXT: entry: -; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i32 [[DELTA:%.*]], 0 -; CHECK-NEXT: [[CMP1:%.*]] = icmp ne i32 [[TAG_TYPE:%.*]], 0 -; CHECK-NEXT: [[OR_COND:%.*]] = and i1 [[TOBOOL]], [[CMP1]] -; CHECK-NEXT: br i1 [[OR_COND]], label [[IF_THEN2:%.*]], label [[IF_END3:%.*]] +; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[DELTA:%.*]], 0 +; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END3:%.*]], !prof !0 +; CHECK: if.then: +; CHECK-NEXT: [[CMP1_NOT:%.*]] = icmp eq i32 [[TAG_TYPE:%.*]], 0 +; CHECK-NEXT: br i1 [[CMP1_NOT]], label [[IF_END3]], label [[IF_THEN2:%.*]] ; CHECK: if.then2: ; CHECK-NEXT: store i8 42, i8* [[IP:%.*]], align 1 ; CHECK-NEXT: br label [[IF_END3]]