From a271ff525dafb2427c1b8a59a39c2f8bc0233862 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Wed, 25 Jul 2018 19:44:19 +0000 Subject: [PATCH] Revert r337904: [IPSCCP] Use PredicateInfo to propagate facts from cmp instructions. I suspect it is causing the clang-stage2-Rthinlto failures. llvm-svn: 337956 --- include/llvm/Transforms/Scalar/SCCP.h | 6 +- lib/Transforms/IPO/SCCP.cpp | 24 +--- lib/Transforms/Scalar/SCCP.cpp | 126 ++---------------- test/Other/new-pm-lto-defaults.ll | 4 +- test/Other/opt-O2-pipeline.ll | 4 - test/Other/opt-O3-pipeline.ll | 4 - test/Other/opt-Os-pipeline.ll | 4 - .../IPConstantProp/musttail-call.ll | 4 +- 8 files changed, 15 insertions(+), 161 deletions(-) diff --git a/include/llvm/Transforms/Scalar/SCCP.h b/include/llvm/Transforms/Scalar/SCCP.h index e434d41d034..2a294c95a17 100644 --- a/include/llvm/Transforms/Scalar/SCCP.h +++ b/include/llvm/Transforms/Scalar/SCCP.h @@ -21,13 +21,11 @@ #ifndef LLVM_TRANSFORMS_SCALAR_SCCP_H #define LLVM_TRANSFORMS_SCALAR_SCCP_H -#include "llvm/ADT/STLExtras.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/Function.h" #include "llvm/IR/Module.h" #include "llvm/IR/PassManager.h" -#include "llvm/Transforms/Utils/PredicateInfo.h" namespace llvm { @@ -39,9 +37,7 @@ public: PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM); }; -bool runIPSCCP( - Module &M, const DataLayout &DL, const TargetLibraryInfo *TLI, - function_ref(Function &)> getPredicateInfo); +bool runIPSCCP(Module &M, const DataLayout &DL, const TargetLibraryInfo *TLI); } // end namespace llvm #endif // LLVM_TRANSFORMS_SCALAR_SCCP_H diff --git a/lib/Transforms/IPO/SCCP.cpp b/lib/Transforms/IPO/SCCP.cpp index e2bb6f185c3..cc53c4b8c46 100644 --- a/lib/Transforms/IPO/SCCP.cpp +++ b/lib/Transforms/IPO/SCCP.cpp @@ -1,5 +1,4 @@ #include "llvm/Transforms/IPO/SCCP.h" -#include "llvm/Analysis/AssumptionCache.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/Transforms/IPO.h" #include "llvm/Transforms/Scalar/SCCP.h" @@ -9,15 +8,7 @@ using namespace llvm; PreservedAnalyses IPSCCPPass::run(Module &M, ModuleAnalysisManager &AM) { const DataLayout &DL = M.getDataLayout(); auto &TLI = AM.getResult(M); - auto &FAM = AM.getResult(M).getManager(); - auto getPredicateInfo = - [&FAM](Function &F) -> std::unique_ptr { - return make_unique(F, - FAM.getResult(F), - FAM.getResult(F)); - }; - - if (!runIPSCCP(M, DL, &TLI, getPredicateInfo)) + if (!runIPSCCP(M, DL, &TLI)) return PreservedAnalyses::all(); return PreservedAnalyses::none(); } @@ -43,20 +34,10 @@ public: const DataLayout &DL = M.getDataLayout(); const TargetLibraryInfo *TLI = &getAnalysis().getTLI(); - - auto getPredicateInfo = - [this](Function &F) -> std::unique_ptr { - return make_unique( - F, this->getAnalysis(F).getDomTree(), - this->getAnalysis().getAssumptionCache(F)); - }; - - return runIPSCCP(M, DL, TLI, getPredicateInfo); + return runIPSCCP(M, DL, TLI); } void getAnalysisUsage(AnalysisUsage &AU) const override { - AU.addRequired(); - AU.addRequired(); AU.addRequired(); } }; @@ -68,7 +49,6 @@ char IPSCCPLegacyPass::ID = 0; INITIALIZE_PASS_BEGIN(IPSCCPLegacyPass, "ipsccp", "Interprocedural Sparse Conditional Constant Propagation", false, false) -INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass) INITIALIZE_PASS_END(IPSCCPLegacyPass, "ipsccp", "Interprocedural Sparse Conditional Constant Propagation", diff --git a/lib/Transforms/Scalar/SCCP.cpp b/lib/Transforms/Scalar/SCCP.cpp index c9d7810c5a3..5e3ddeda2d4 100644 --- a/lib/Transforms/Scalar/SCCP.cpp +++ b/lib/Transforms/Scalar/SCCP.cpp @@ -55,7 +55,6 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Transforms/Scalar.h" -#include "llvm/Transforms/Utils/PredicateInfo.h" #include #include #include @@ -247,21 +246,7 @@ class SCCPSolver : public InstVisitor { using Edge = std::pair; DenseSet KnownFeasibleEdges; - DenseMap> PredInfos; - DenseMap> AdditionalUsers; - public: - void addPredInfo(Function &F, std::unique_ptr PI) { - PredInfos[&F] = std::move(PI); - } - - const PredicateBase *getPredicateInfoFor(Instruction *I) { - auto PI = PredInfos.find(I->getFunction()); - if (PI == PredInfos.end()) - return nullptr; - return PI->second->getPredicateInfoFor(I); - } - SCCPSolver(const DataLayout &DL, const TargetLibraryInfo *tli) : DL(DL), TLI(tli) {} @@ -573,26 +558,6 @@ private: visit(*I); } - // Add U as additional user of V. - void addAdditionalUser(Value *V, User *U) { - auto Iter = AdditionalUsers.insert({V, {}}); - Iter.first->second.insert(U); - } - - // Mark I's users as changed, including AdditionalUsers. - void markUsersAsChanged(Value *I) { - for (User *U : I->users()) - if (auto *UI = dyn_cast(U)) - OperandChangedState(UI); - - auto Iter = AdditionalUsers.find(I); - if (Iter != AdditionalUsers.end()) { - for (User *U : Iter->second) - if (auto *UI = dyn_cast(U)) - OperandChangedState(UI); - } - } - private: friend class InstVisitor; @@ -1154,65 +1119,6 @@ void SCCPSolver::visitCallSite(CallSite CS) { Function *F = CS.getCalledFunction(); Instruction *I = CS.getInstruction(); - if (auto *II = dyn_cast(I)) { - if (II->getIntrinsicID() == Intrinsic::ssa_copy) { - if (ValueState[I].isOverdefined()) - return; - - auto *PI = getPredicateInfoFor(I); - if (!PI) - return; - - auto *PBranch = dyn_cast(getPredicateInfoFor(I)); - if (!PBranch) { - mergeInValue(ValueState[I], I, getValueState(PI->OriginalOp)); - return; - } - - Value *CopyOf = I->getOperand(0); - Value *Cond = PBranch->Condition; - - // Everything below relies on the condition being a comparison. - auto *Cmp = dyn_cast(Cond); - if (!Cmp) { - mergeInValue(ValueState[I], I, getValueState(PI->OriginalOp)); - return; - } - - Value *CmpOp0 = Cmp->getOperand(0); - Value *CmpOp1 = Cmp->getOperand(1); - if (CopyOf != CmpOp0 && CopyOf != CmpOp1) { - mergeInValue(ValueState[I], I, getValueState(PI->OriginalOp)); - return; - } - - if (CmpOp0 != CopyOf) - std::swap(CmpOp0, CmpOp1); - - LatticeVal OriginalVal = getValueState(CopyOf); - LatticeVal EqVal = getValueState(CmpOp1); - LatticeVal &IV = ValueState[I]; - if (PBranch->TrueEdge && Cmp->getPredicate() == CmpInst::ICMP_EQ) { - addAdditionalUser(CmpOp1, I); - if (OriginalVal.isConstant()) - mergeInValue(IV, I, OriginalVal); - else - mergeInValue(IV, I, EqVal); - return; - } - if (!PBranch->TrueEdge && Cmp->getPredicate() == CmpInst::ICMP_NE) { - addAdditionalUser(CmpOp1, I); - if (OriginalVal.isConstant()) - mergeInValue(IV, I, OriginalVal); - else - mergeInValue(IV, I, EqVal); - return; - } - - return (void)mergeInValue(IV, I, getValueState(PBranch->OriginalOp)); - } - } - // The common case is that we aren't tracking the callee, either because we // are not doing interprocedural analysis or the callee is indirect, or is // external. Handle these cases first. @@ -1332,7 +1238,9 @@ void SCCPSolver::Solve() { // since all of its users will have already been marked as overdefined // Update all of the users of this instruction's value. // - markUsersAsChanged(I); + for (User *U : I->users()) + if (auto *UI = dyn_cast(U)) + OperandChangedState(UI); } // Process the instruction work list. @@ -1349,7 +1257,9 @@ void SCCPSolver::Solve() { // Update all of the users of this instruction's value. // if (I->getType()->isStructTy() || !getValueState(I).isOverdefined()) - markUsersAsChanged(I); + for (User *U : I->users()) + if (auto *UI = dyn_cast(U)) + OperandChangedState(UI); } // Process the basic block work list. @@ -1888,9 +1798,8 @@ static void findReturnsToZap(Function &F, } } -bool llvm::runIPSCCP( - Module &M, const DataLayout &DL, const TargetLibraryInfo *TLI, - function_ref(Function &)> getPredicateInfo) { +bool llvm::runIPSCCP(Module &M, const DataLayout &DL, + const TargetLibraryInfo *TLI) { SCCPSolver Solver(DL, TLI); // Loop over all functions, marking arguments to those with their addresses @@ -1899,7 +1808,6 @@ bool llvm::runIPSCCP( if (F.isDeclaration()) continue; - Solver.addPredInfo(F, getPredicateInfo(F)); // Determine if we can track the function's return values. If so, add the // function to the solver's set of return-tracked functions. if (canTrackReturnsInterprocedurally(&F)) @@ -2052,24 +1960,6 @@ bool llvm::runIPSCCP( F.getBasicBlockList().erase(DeadBB); } BlocksToErase.clear(); - - for (BasicBlock &BB : F) { - for (BasicBlock::iterator BI = BB.begin(), E = BB.end(); BI != E;) { - Instruction *Inst = &*BI++; - if (const PredicateBase *PI = Solver.getPredicateInfoFor(Inst)) { - if (auto *II = dyn_cast(Inst)) { - if (II->getIntrinsicID() == Intrinsic::ssa_copy) { - Value *Op = II->getOperand(0); - Inst->replaceAllUsesWith(Op); - Inst->eraseFromParent(); - continue; - } - } - Inst->replaceAllUsesWith(PI->OriginalOp); - Inst->eraseFromParent(); - } - } - } } // If we inferred constant or undef return values for a function, we replaced diff --git a/test/Other/new-pm-lto-defaults.ll b/test/Other/new-pm-lto-defaults.ll index e70f6d0fc8f..5bb4d9a4eac 100644 --- a/test/Other/new-pm-lto-defaults.ll +++ b/test/Other/new-pm-lto-defaults.ll @@ -41,8 +41,6 @@ ; CHECK-O2-NEXT: Running analysis: ProfileSummaryAnalysis ; CHECK-O2-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis ; CHECK-O2-NEXT: Running pass: IPSCCPPass -; CHECK-O2-DAG: Running analysis: AssumptionAnalysis on foo -; CHECK-O2-DAG: Running analysis: DominatorTreeAnalysis on foo ; CHECK-O2-NEXT: Running pass: CalledValuePropagationPass ; CHECK-O-NEXT: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}PostOrderFunctionAttrsPass> ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}SCC @@ -59,6 +57,8 @@ ; CHECK-O1-NEXT: Running pass: LowerTypeTestsPass ; CHECK-O2-NEXT: Running pass: GlobalOptPass ; CHECK-O2-NEXT: Running pass: ModuleToFunctionPassAdaptor<{{.*}}PromotePass> +; CHECK-O2-NEXT: Running analysis: DominatorTreeAnalysis +; CHECK-O2-NEXT: Running analysis: AssumptionAnalysis ; CHECK-O2-NEXT: Running pass: ConstantMergePass ; CHECK-O2-NEXT: Running pass: DeadArgumentEliminationPass ; CHECK-O2-NEXT: Running pass: ModuleToFunctionPassAdaptor<{{.*}}PassManager{{.*}}> diff --git a/test/Other/opt-O2-pipeline.ll b/test/Other/opt-O2-pipeline.ll index 71b3003da82..2ebb6ed909f 100644 --- a/test/Other/opt-O2-pipeline.ll +++ b/test/Other/opt-O2-pipeline.ll @@ -28,7 +28,6 @@ ; CHECK-NEXT: Force set function attributes ; CHECK-NEXT: Infer set function attributes ; CHECK-NEXT: Interprocedural Sparse Conditional Constant Propagation -; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() ; CHECK-NEXT: Called Value Propagation ; CHECK-NEXT: Global Variable Optimizer ; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() @@ -271,9 +270,6 @@ ; CHECK-NEXT: Module Verifier ; CHECK-NEXT: Bitcode Writer ; CHECK-NEXT: Pass Arguments: -; CHECK-NEXT: FunctionPass Manager -; CHECK-NEXT: Dominator Tree Construction -; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: FunctionPass Manager ; CHECK-NEXT: Dominator Tree Construction diff --git a/test/Other/opt-O3-pipeline.ll b/test/Other/opt-O3-pipeline.ll index b82a4967755..d9ffc96d434 100644 --- a/test/Other/opt-O3-pipeline.ll +++ b/test/Other/opt-O3-pipeline.ll @@ -30,7 +30,6 @@ ; CHECK-NEXT: FunctionPass Manager ; CHECK-NEXT: Call-site splitting ; CHECK-NEXT: Interprocedural Sparse Conditional Constant Propagation -; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() ; CHECK-NEXT: Called Value Propagation ; CHECK-NEXT: Global Variable Optimizer ; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() @@ -275,9 +274,6 @@ ; CHECK-NEXT: Module Verifier ; CHECK-NEXT: Bitcode Writer ; CHECK-NEXT: Pass Arguments: -; CHECK-NEXT: FunctionPass Manager -; CHECK-NEXT: Dominator Tree Construction -; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: FunctionPass Manager ; CHECK-NEXT: Dominator Tree Construction diff --git a/test/Other/opt-Os-pipeline.ll b/test/Other/opt-Os-pipeline.ll index 1d42abb8ff3..58bf62ffc9c 100644 --- a/test/Other/opt-Os-pipeline.ll +++ b/test/Other/opt-Os-pipeline.ll @@ -28,7 +28,6 @@ ; CHECK-NEXT: Force set function attributes ; CHECK-NEXT: Infer set function attributes ; CHECK-NEXT: Interprocedural Sparse Conditional Constant Propagation -; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() ; CHECK-NEXT: Called Value Propagation ; CHECK-NEXT: Global Variable Optimizer ; CHECK-NEXT: Unnamed pass: implement Pass::getPassName() @@ -258,9 +257,6 @@ ; CHECK-NEXT: Module Verifier ; CHECK-NEXT: Bitcode Writer ; CHECK-NEXT: Pass Arguments: -; CHECK-NEXT: FunctionPass Manager -; CHECK-NEXT: Dominator Tree Construction -; CHECK-NEXT: Pass Arguments: ; CHECK-NEXT: Target Library Information ; CHECK-NEXT: FunctionPass Manager ; CHECK-NEXT: Dominator Tree Construction diff --git a/test/Transforms/IPConstantProp/musttail-call.ll b/test/Transforms/IPConstantProp/musttail-call.ll index 567ca408099..75877585710 100644 --- a/test/Transforms/IPConstantProp/musttail-call.ll +++ b/test/Transforms/IPConstantProp/musttail-call.ll @@ -9,7 +9,7 @@ define i8* @start(i8 %v) { %c1 = icmp eq i8 %v, 0 br i1 %c1, label %true, label %false true: - ; CHECK: %ca = musttail call i8* @side_effects(i8 0) + ; CHECK: %ca = musttail call i8* @side_effects(i8 %v) ; CHECK: ret i8* %ca %ca = musttail call i8* @side_effects(i8 %v) ret i8* %ca @@ -34,7 +34,7 @@ define internal i8* @side_effects(i8 %v) { ; is always `null`. ; The call can't be removed due to `external` call above, though. - ; CHECK: %ca = musttail call i8* @start(i8 0) + ; CHECK: %ca = musttail call i8* @start(i8 %v) %ca = musttail call i8* @start(i8 %v) ; Thus the result must be returned anyway