From b80daf0b48c6be27812c95904298fac0c9bdc4e2 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Sun, 22 Oct 2017 19:10:07 +0000 Subject: [PATCH] [SimplifyCFG] delay switch condition forwarding to -latesimplifycfg As discussed in D39011: https://reviews.llvm.org/D39011 ...replacing constants with a variable is inverting the transform done by other IR passes, so we definitely don't want to do this early. In fact, it's questionable whether this transform belongs in SimplifyCFG at all. I'll look at moving this to codegen as a follow-up step. llvm-svn: 316298 --- llvm/include/llvm/Transforms/Utils/Local.h | 6 ++++-- llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp | 18 ++++++++++++------ llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 2 +- .../SimplifyCFG/ForwardSwitchConditionToPHI.ll | 2 +- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index af00a6c..f66a05e 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -60,14 +60,16 @@ class TargetTransformInfo; /// replaced by lookup tables and selects. struct SimplifyCFGOptions { int BonusInstThreshold; + bool ForwardSwitchCondToPhi; bool ConvertSwitchToLookupTable; bool NeedCanonicalLoop; AssumptionCache *AC; - SimplifyCFGOptions(int BonusThreshold = 1, bool SwitchToLookup = false, - bool CanonicalLoops = true, + SimplifyCFGOptions(int BonusThreshold = 1, bool ForwardSwitchCond = false, + bool SwitchToLookup = false, bool CanonicalLoops = true, AssumptionCache *AssumpCache = nullptr) : BonusInstThreshold(BonusThreshold), + ForwardSwitchCondToPhi(ForwardSwitchCond), ConvertSwitchToLookupTable(SwitchToLookup), NeedCanonicalLoop(CanonicalLoops), AC(AssumpCache) {} }; diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp index 723619f..6f38e5d 100644 --- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp +++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp @@ -179,8 +179,10 @@ static bool simplifyFunctionCFG(Function &F, const TargetTransformInfo &TTI, return true; } +// FIXME: The new pass manager always creates a "late" simplifycfg pass using +// this default constructor. SimplifyCFGPass::SimplifyCFGPass() - : Options(UserBonusInstThreshold, true, false) {} + : Options(UserBonusInstThreshold, true, true, false) {} SimplifyCFGPass::SimplifyCFGPass(const SimplifyCFGOptions &PassOptions) : Options(PassOptions) {} @@ -200,12 +202,15 @@ namespace { struct BaseCFGSimplifyPass : public FunctionPass { std::function PredicateFtor; int BonusInstThreshold; + bool ForwardSwitchCondToPhi; bool ConvertSwitchToLookupTable; bool KeepCanonicalLoops; - BaseCFGSimplifyPass(int T, bool ConvertSwitch, bool KeepLoops, + BaseCFGSimplifyPass(int T, bool ForwardSwitchCond, bool ConvertSwitch, + bool KeepLoops, std::function Ftor, char &ID) : FunctionPass(ID), PredicateFtor(std::move(Ftor)), + ForwardSwitchCondToPhi(ForwardSwitchCond), ConvertSwitchToLookupTable(ConvertSwitch), KeepCanonicalLoops(KeepLoops) { BonusInstThreshold = (T == -1) ? UserBonusInstThreshold : T; @@ -219,8 +224,9 @@ struct BaseCFGSimplifyPass : public FunctionPass { const TargetTransformInfo &TTI = getAnalysis().getTTI(F); return simplifyFunctionCFG(F, TTI, - {BonusInstThreshold, ConvertSwitchToLookupTable, - KeepCanonicalLoops, AC}); + {BonusInstThreshold, ForwardSwitchCondToPhi, + ConvertSwitchToLookupTable, KeepCanonicalLoops, + AC}); } void getAnalysisUsage(AnalysisUsage &AU) const override { @@ -235,7 +241,7 @@ struct CFGSimplifyPass : public BaseCFGSimplifyPass { CFGSimplifyPass(int T = -1, std::function Ftor = nullptr) - : BaseCFGSimplifyPass(T, false, true, Ftor, ID) { + : BaseCFGSimplifyPass(T, false, false, true, Ftor, ID) { initializeCFGSimplifyPassPass(*PassRegistry::getPassRegistry()); } }; @@ -245,7 +251,7 @@ struct LateCFGSimplifyPass : public BaseCFGSimplifyPass { LateCFGSimplifyPass(int T = -1, std::function Ftor = nullptr) - : BaseCFGSimplifyPass(T, true, false, Ftor, ID) { + : BaseCFGSimplifyPass(T, true, true, false, Ftor, ID) { initializeLateCFGSimplifyPassPass(*PassRegistry::getPassRegistry()); } }; diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 42f2bf2..954d633 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -5563,7 +5563,7 @@ bool SimplifyCFGOpt::SimplifySwitch(SwitchInst *SI, IRBuilder<> &Builder) { if (switchToSelect(SI, Builder, DL, TTI)) return simplifyCFG(BB, TTI, Options) | true; - if (ForwardSwitchConditionToPHI(SI)) + if (Options.ForwardSwitchCondToPhi && ForwardSwitchConditionToPHI(SI)) return simplifyCFG(BB, TTI, Options) | true; // The conversion from switch to lookup tables results in difficult-to-analyze diff --git a/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll b/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll index d51d24a..4dcccef 100644 --- a/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll +++ b/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll @@ -1,5 +1,5 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py -; RUN: opt < %s -simplifycfg -S | FileCheck %s +; RUN: opt < %s -latesimplifycfg -S | FileCheck %s ; PR10131 -- 2.7.4