From 109cab44506140a5671d9b0b1643c9bd5558e6cb Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Fri, 28 Aug 2015 06:52:00 +0000 Subject: [PATCH] Revert r246244 and r246243 These two commits cause clang/llvm bootstrap to hang. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@246279 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Transforms/Utils/Local.h | 4 - lib/IR/Dominators.cpp | 6 +- lib/Transforms/Scalar/GVN.cpp | 104 +++---------------- lib/Transforms/Utils/Local.cpp | 20 ---- test/Transforms/GVN/assume-equal.ll | 137 -------------------------- 5 files changed, 13 insertions(+), 258 deletions(-) delete mode 100644 test/Transforms/GVN/assume-equal.ll diff --git a/include/llvm/Transforms/Utils/Local.h b/include/llvm/Transforms/Utils/Local.h index 5445c1b0e4f..a1bb367ac7b 100644 --- a/include/llvm/Transforms/Utils/Local.h +++ b/include/llvm/Transforms/Utils/Local.h @@ -291,10 +291,6 @@ void combineMetadata(Instruction *K, const Instruction *J, ArrayRef Kn /// the given edge. Returns the number of replacements made. unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT, const BasicBlockEdge &Edge); -/// \brief Replace each use of 'From' with 'To' if that use is dominated by -/// the given BasicBlock. Returns the number of replacements made. -unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT, - const BasicBlock *BB); } // End llvm namespace #endif diff --git a/lib/IR/Dominators.cpp b/lib/IR/Dominators.cpp index d94e31d4875..775bd92f2ed 100644 --- a/lib/IR/Dominators.cpp +++ b/lib/IR/Dominators.cpp @@ -147,8 +147,7 @@ bool DominatorTree::dominates(const BasicBlockEdge &BBE, // Assert that we have a single edge. We could handle them by simply // returning false, but since isSingleEdge is linear on the number of // edges, the callers can normally handle them more efficiently. - assert(BBE.isSingleEdge() && - "This function is not efficient in handling multiple edges"); + assert(BBE.isSingleEdge()); // If the BB the edge ends in doesn't dominate the use BB, then the // edge also doesn't. @@ -198,8 +197,7 @@ bool DominatorTree::dominates(const BasicBlockEdge &BBE, const Use &U) const { // Assert that we have a single edge. We could handle them by simply // returning false, but since isSingleEdge is linear on the number of // edges, the callers can normally handle them more efficiently. - assert(BBE.isSingleEdge() && - "This function is not efficient in handling multiple edges"); + assert(BBE.isSingleEdge()); Instruction *UserInst = cast(U.getUser()); // A PHI in the end of the edge is dominated by it. diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp index 092cb69f17c..d898b1796b6 100644 --- a/lib/Transforms/Scalar/GVN.cpp +++ b/lib/Transforms/Scalar/GVN.cpp @@ -608,10 +608,6 @@ namespace { DenseMap LeaderTable; BumpPtrAllocator TableAllocator; - // Block-local map of equivalent values to their leader, does not - // propagate to any successors. Entries added mid-block are applied - // to the remaining instructions in the block. - SmallMapVector ReplaceWithConstMap; SmallVector InstrsToErase; typedef SmallVector LoadDepVect; @@ -703,7 +699,6 @@ namespace { // Helper functions of redundant load elimination bool processLoad(LoadInst *L); bool processNonLocalLoad(LoadInst *L); - bool processAssumeIntrinsic(IntrinsicInst *II); void AnalyzeLoadAvailability(LoadInst *LI, LoadDepVect &Deps, AvailValInBlkVect &ValuesPerBlock, UnavailBlkVect &UnavailableBlocks); @@ -724,9 +719,7 @@ namespace { void verifyRemoved(const Instruction *I) const; bool splitCriticalEdges(); BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ); - bool replaceOperandsWithConsts(Instruction *I) const; - bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root, - bool DominatesByEdge); + bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root); bool processFoldableCondBr(BranchInst *BI); void addDeadBlock(BasicBlock *BB); void assignValNumForDeadCode(); @@ -1767,46 +1760,6 @@ bool GVN::processNonLocalLoad(LoadInst *LI) { return PerformLoadPRE(LI, ValuesPerBlock, UnavailableBlocks); } -bool GVN::processAssumeIntrinsic(IntrinsicInst *IntrinsicI) { - assert(IntrinsicI->getIntrinsicID() == Intrinsic::assume && - "This function can only be called with llvm.assume intrinsic"); - Value *V = IntrinsicI->getArgOperand(0); - Constant *True = ConstantInt::getTrue(V->getContext()); - bool Changed = false; - for (BasicBlock *Successor : successors(IntrinsicI->getParent())) { - BasicBlockEdge Edge(IntrinsicI->getParent(), Successor); - - // This property is only true in dominated successors, propagateEquality - // will check dominance for us. - Changed |= propagateEquality(V, True, Edge, false); - } - // We can replace assume value with true, which covers cases like this: - // call void @llvm.assume(i1 %cmp) - // br i1 %cmp, label %bb1, label %bb2 ; will change %cmp to true - ReplaceWithConstMap[V] = True; - - // If one of *cmp *eq operand is const, adding it to map will cover this: - // %cmp = fcmp oeq float 3.000000e+00, %0 ; const on lhs could happen - // call void @llvm.assume(i1 %cmp) - // ret float %0 ; will change it to ret float 3.000000e+00 - if (auto *CmpI = dyn_cast(V)) { - if (CmpI->getPredicate() == CmpInst::Predicate::ICMP_EQ || - CmpI->getPredicate() == CmpInst::Predicate::FCMP_OEQ || - (CmpI->getPredicate() == CmpInst::Predicate::FCMP_UEQ && - CmpI->getFastMathFlags().noNaNs())) { - Value *CmpLHS = CmpI->getOperand(0); - Value *CmpRHS = CmpI->getOperand(1); - if (isa(CmpLHS)) - std::swap(CmpLHS, CmpRHS); - auto *RHSConst = dyn_cast(CmpRHS); - - // If only one operand is constant. - if (RHSConst != nullptr && !isa(CmpLHS)) - ReplaceWithConstMap[CmpLHS] = RHSConst; - } - } - return Changed; -} static void patchReplacementInstruction(Instruction *I, Value *Repl) { // Patch the replacement so that it is not more restrictive than the value @@ -2079,27 +2032,11 @@ static bool isOnlyReachableViaThisEdge(const BasicBlockEdge &E, return Pred != nullptr; } -// Tries to replace instruction with const, using information from -// ReplaceWithConstMap. -bool GVN::replaceOperandsWithConsts(Instruction *Instr) const { - bool Changed = false; - for (unsigned OpNum = 0; OpNum < Instr->getNumOperands(); ++OpNum) { - Value *Operand = Instr->getOperand(OpNum); - auto it = ReplaceWithConstMap.find(Operand); - if (it != ReplaceWithConstMap.end()) { - Instr->setOperand(OpNum, it->second); - Changed = true; - } - } - return Changed; -} - /// The given values are known to be equal in every block /// dominated by 'Root'. Exploit this, for example by replacing 'LHS' with /// 'RHS' everywhere in the scope. Returns whether a change was made. -/// If DominatesByEdge is false, then it means that it is dominated by Root.End. -bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root, - bool DominatesByEdge) { +bool GVN::propagateEquality(Value *LHS, Value *RHS, + const BasicBlockEdge &Root) { SmallVector, 4> Worklist; Worklist.push_back(std::make_pair(LHS, RHS)); bool Changed = false; @@ -2111,13 +2048,11 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root, std::pair Item = Worklist.pop_back_val(); LHS = Item.first; RHS = Item.second; - if (LHS == RHS) - continue; + if (LHS == RHS) continue; assert(LHS->getType() == RHS->getType() && "Equality but unequal types!"); // Don't try to propagate equalities between constants. - if (isa(LHS) && isa(RHS)) - continue; + if (isa(LHS) && isa(RHS)) continue; // Prefer a constant on the right-hand side, or an Argument if no constants. if (isa(LHS) || (isa(LHS) && !isa(RHS))) @@ -2156,11 +2091,7 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root, // LHS always has at least one use that is not dominated by Root, this will // never do anything if LHS has only one use. if (!LHS->hasOneUse()) { - unsigned NumReplacements = - DominatesByEdge - ? replaceDominatedUsesWith(LHS, RHS, *DT, Root) - : replaceDominatedUsesWith(LHS, RHS, *DT, Root.getEnd()); - + unsigned NumReplacements = replaceDominatedUsesWith(LHS, RHS, *DT, Root); Changed |= NumReplacements > 0; NumGVNEqProp += NumReplacements; } @@ -2232,10 +2163,7 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root, Value *NotCmp = findLeader(Root.getEnd(), Num); if (NotCmp && isa(NotCmp)) { unsigned NumReplacements = - DominatesByEdge - ? replaceDominatedUsesWith(NotCmp, NotVal, *DT, Root) - : replaceDominatedUsesWith(NotCmp, NotVal, *DT, - Root.getEnd()); + replaceDominatedUsesWith(NotCmp, NotVal, *DT, Root); Changed |= NumReplacements > 0; NumGVNEqProp += NumReplacements; } @@ -2275,10 +2203,6 @@ bool GVN::processInstruction(Instruction *I) { return true; } - if (IntrinsicInst *IntrinsicI = dyn_cast(I)) - if (IntrinsicI->getIntrinsicID() == Intrinsic::assume) - return processAssumeIntrinsic(IntrinsicI); - if (LoadInst *LI = dyn_cast(I)) { if (processLoad(LI)) return true; @@ -2309,11 +2233,11 @@ bool GVN::processInstruction(Instruction *I) { Value *TrueVal = ConstantInt::getTrue(TrueSucc->getContext()); BasicBlockEdge TrueE(Parent, TrueSucc); - Changed |= propagateEquality(BranchCond, TrueVal, TrueE, true); + Changed |= propagateEquality(BranchCond, TrueVal, TrueE); Value *FalseVal = ConstantInt::getFalse(FalseSucc->getContext()); BasicBlockEdge FalseE(Parent, FalseSucc); - Changed |= propagateEquality(BranchCond, FalseVal, FalseE, true); + Changed |= propagateEquality(BranchCond, FalseVal, FalseE); return Changed; } @@ -2335,7 +2259,7 @@ bool GVN::processInstruction(Instruction *I) { // If there is only a single edge, propagate the case value into it. if (SwitchEdges.lookup(Dst) == 1) { BasicBlockEdge E(Parent, Dst); - Changed |= propagateEquality(SwitchCond, i.getCaseValue(), E, true); + Changed |= propagateEquality(SwitchCond, i.getCaseValue(), E); } } return Changed; @@ -2343,8 +2267,7 @@ bool GVN::processInstruction(Instruction *I) { // Instructions with void type don't return a value, so there's // no point in trying to find redundancies in them. - if (I->getType()->isVoidTy()) - return false; + if (I->getType()->isVoidTy()) return false; uint32_t NextNum = VN.getNextUnusedValueNumber(); unsigned Num = VN.lookup_or_add(I); @@ -2451,15 +2374,10 @@ bool GVN::processBlock(BasicBlock *BB) { if (DeadBlocks.count(BB)) return false; - // Clearing map before every BB because it can be used only for single BB. - ReplaceWithConstMap.clear(); bool ChangedFunction = false; for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); BI != BE;) { - if (!ReplaceWithConstMap.empty()) - ChangedFunction |= replaceOperandsWithConsts(BI); - ChangedFunction |= processInstruction(BI); if (InstrsToErase.empty()) { ++BI; diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index 364651bda3c..8c29ed50b45 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -1349,23 +1349,3 @@ unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To, } return Count; } - -unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To, - DominatorTree &DT, - const BasicBlock *BB) { - assert(From->getType() == To->getType()); - - unsigned Count = 0; - for (Value::use_iterator UI = From->use_begin(), UE = From->use_end(); - UI != UE;) { - Use &U = *UI++; - auto *I = cast(U.getUser()); - if (DT.dominates(BB, I->getParent())) { - U.set(To); - DEBUG(dbgs() << "Replace dominated use of '" << From->getName() << "' as " - << *To << " in " << *U << "\n"); - ++Count; - } - } - return Count; -} diff --git a/test/Transforms/GVN/assume-equal.ll b/test/Transforms/GVN/assume-equal.ll deleted file mode 100644 index 0e4df969f43..00000000000 --- a/test/Transforms/GVN/assume-equal.ll +++ /dev/null @@ -1,137 +0,0 @@ -; RUN: opt < %s -gvn -S | FileCheck %s - -%struct.A = type { i32 (...)** } -@_ZTV1A = available_externally unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast (i8** @_ZTI1A to i8*), i8* bitcast (i32 (%struct.A*)* @_ZN1A3fooEv to i8*), i8* bitcast (i32 (%struct.A*)* @_ZN1A3barEv to i8*)], align 8 -@_ZTI1A = external constant i8* - -; Checks if indirect calls can be replaced with direct -; assuming that %vtable == @_ZTV1A (with alignment). -; Checking const propagation across other BBs -; CHECK-LABEL: define void @_Z1gb( - -define void @_Z1gb(i1 zeroext %p) { -entry: - %call = tail call noalias i8* @_Znwm(i64 8) #4 - %0 = bitcast i8* %call to %struct.A* - tail call void @_ZN1AC1Ev(%struct.A* %0) #1 - %1 = bitcast i8* %call to i8*** - %vtable = load i8**, i8*** %1, align 8 - %cmp.vtables = icmp eq i8** %vtable, getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTV1A, i64 0, i64 2) - tail call void @llvm.assume(i1 %cmp.vtables) - br i1 %p, label %if.then, label %if.else - -if.then: ; preds = %entry - %vtable1.cast = bitcast i8** %vtable to i32 (%struct.A*)** - %2 = load i32 (%struct.A*)*, i32 (%struct.A*)** %vtable1.cast, align 8 - - ; CHECK: call i32 @_ZN1A3fooEv( - %call2 = tail call i32 %2(%struct.A* %0) #1 - - br label %if.end - -if.else: ; preds = %entry - %vfn47 = getelementptr inbounds i8*, i8** %vtable, i64 1 - %vfn4 = bitcast i8** %vfn47 to i32 (%struct.A*)** - - ; CHECK: call i32 @_ZN1A3barEv( - %3 = load i32 (%struct.A*)*, i32 (%struct.A*)** %vfn4, align 8 - - %call5 = tail call i32 %3(%struct.A* %0) #1 - br label %if.end - -if.end: ; preds = %if.else, %if.then - ret void -} - -; Checking const propagation in the same BB -; CHECK-LABEL: define i32 @main() - -define i32 @main() { -entry: - %call = tail call noalias i8* @_Znwm(i64 8) - %0 = bitcast i8* %call to %struct.A* - tail call void @_ZN1AC1Ev(%struct.A* %0) - %1 = bitcast i8* %call to i8*** - %vtable = load i8**, i8*** %1, align 8 - %cmp.vtables = icmp eq i8** %vtable, getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTV1A, i64 0, i64 2) - tail call void @llvm.assume(i1 %cmp.vtables) - %vtable1.cast = bitcast i8** %vtable to i32 (%struct.A*)** - - ; CHECK: call i32 @_ZN1A3fooEv( - %2 = load i32 (%struct.A*)*, i32 (%struct.A*)** %vtable1.cast, align 8 - - %call2 = tail call i32 %2(%struct.A* %0) - ret i32 0 -} - -; This tests checks const propatation with fcmp instruction. -; CHECK-LABEL: define float @_Z1gf(float %p) - -define float @_Z1gf(float %p) { -entry: - %p.addr = alloca float, align 4 - %f = alloca float, align 4 - store float %p, float* %p.addr, align 4 - - store float 3.000000e+00, float* %f, align 4 - %0 = load float, float* %p.addr, align 4 - %1 = load float, float* %f, align 4 - %cmp = fcmp oeq float %1, %0 ; note const on lhs - call void @llvm.assume(i1 %cmp) - - ; CHECK: ret float 3.000000e+00 - ret float %0 -} - -; CHECK-LABEL: define float @_Z1hf(float %p) - -define float @_Z1hf(float %p) { -entry: - %p.addr = alloca float, align 4 - store float %p, float* %p.addr, align 4 - - %0 = load float, float* %p.addr, align 4 - %cmp = fcmp nnan ueq float %0, 3.000000e+00 - call void @llvm.assume(i1 %cmp) - - ; CHECK: ret float 3.000000e+00 - ret float %0 -} - -; CHECK-LABEL: define float @_Z1if(float %p) -define float @_Z1if(float %p) { -entry: - %p.addr = alloca float, align 4 - store float %p, float* %p.addr, align 4 - - %0 = load float, float* %p.addr, align 4 - %cmp = fcmp ueq float %0, 3.000000e+00 ; no nnan flag - can't propagate - call void @llvm.assume(i1 %cmp) - - ; CHECK-NOT: ret float 3.000000e+00 - ret float %0 -} - -; This test checks if constant propagation works for multiple node edges -; CHECK-LABEL: define i32 @_Z1ii(i32 %p) -define i32 @_Z1ii(i32 %p) { -entry: - %cmp = icmp eq i32 %p, 42 - call void @llvm.assume(i1 %cmp) - - ; CHECK: br i1 true, label %bb2, label %bb2 - br i1 %cmp, label %bb2, label %bb2 -bb2: - ; CHECK: br i1 true, label %bb2, label %bb2 - br i1 %cmp, label %bb2, label %bb2 - - ; CHECK: ret i32 42 - ret i32 %p -} - -declare noalias i8* @_Znwm(i64) -declare void @_ZN1AC1Ev(%struct.A*) -declare void @llvm.assume(i1) -declare i32 @_ZN1A3fooEv(%struct.A*) -declare i32 @_ZN1A3barEv(%struct.A*) -