From 7b861f08cd007bcdb18c6d2895ecc4b17b686d06 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Wed, 1 Nov 2017 19:49:20 +0000 Subject: [PATCH] Revert 317016 and 317048 The former appears to have introduced a miscompile in a stage2 clang build. Revert so I can investigate offline. llvm-svn: 317116 --- llvm/lib/Transforms/Utils/SimplifyIndVar.cpp | 94 +++++++++++--------- 1 file changed, 50 insertions(+), 44 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp index 66e633518221..fce7f8b81bac 100644 --- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp @@ -83,6 +83,7 @@ namespace { bool eliminateOverflowIntrinsic(CallInst *CI); bool eliminateIVUser(Instruction *UseInst, Instruction *IVOperand); + bool makeIVComparisonInvariant(ICmpInst *ICmp, Value *IVOperand); void eliminateIVComparison(ICmpInst *ICmp, Value *IVOperand); void simplifyIVRemainder(BinaryOperator *Rem, Value *IVOperand, bool IsSigned); @@ -92,16 +93,6 @@ namespace { bool eliminateSDiv(BinaryOperator *SDiv); bool strengthenOverflowingOperation(BinaryOperator *OBO, Value *IVOperand); bool strengthenRightShift(BinaryOperator *BO, Value *IVOperand); - - - private: - // Wrapped around ScalarEvolution::isLoopInvariantPredicate which returns - // true only when a free expansion is available. - bool isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred, - const SCEV *LHS, const SCEV *RHS, const Loop *L, - const SmallDenseMap &FreeExpansions, - ICmpInst::Predicate &InvariantPred, - Value *&LHSV, Value *& RHSV); }; } @@ -171,24 +162,62 @@ Value *SimplifyIndvar::foldIVUser(Instruction *UseInst, Instruction *IVOperand) return IVSrc; } -bool SimplifyIndvar::isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred, - const SCEV *LHS, const SCEV *RHS, const Loop *L, - const SmallDenseMap &FreeExpansions, - ICmpInst::Predicate &InvariantPred, - Value *&LHSV, Value *& RHSV) { +bool SimplifyIndvar::makeIVComparisonInvariant(ICmpInst *ICmp, + Value *IVOperand) { + unsigned IVOperIdx = 0; + ICmpInst::Predicate Pred = ICmp->getPredicate(); + if (IVOperand != ICmp->getOperand(0)) { + // Swapped + assert(IVOperand == ICmp->getOperand(1) && "Can't find IVOperand"); + IVOperIdx = 1; + Pred = ICmpInst::getSwappedPredicate(Pred); + } + // Get the SCEVs for the ICmp operands (in the specific context of the + // current loop) + const Loop *ICmpLoop = LI->getLoopFor(ICmp->getParent()); + const SCEV *S = SE->getSCEVAtScope(ICmp->getOperand(IVOperIdx), ICmpLoop); + const SCEV *X = SE->getSCEVAtScope(ICmp->getOperand(1 - IVOperIdx), ICmpLoop); + + ICmpInst::Predicate InvariantPredicate; const SCEV *InvariantLHS, *InvariantRHS; - if (!SE->isLoopInvariantPredicate(Pred, LHS, RHS, L, InvariantPred, + + auto *PN = dyn_cast(IVOperand); + if (!PN) + return false; + if (!SE->isLoopInvariantPredicate(Pred, S, X, L, InvariantPredicate, InvariantLHS, InvariantRHS)) return false; // Rewrite the comparison to a loop invariant comparison if it can be done // cheaply, where cheaply means "we don't need to emit any new // instructions". - LHSV = FreeExpansions.lookup(InvariantLHS); - RHSV = FreeExpansions.lookup(InvariantRHS); - return (LHSV && RHSV); + SmallDenseMap CheapExpansions; + CheapExpansions[S] = ICmp->getOperand(IVOperIdx); + CheapExpansions[X] = ICmp->getOperand(1 - IVOperIdx); + + // TODO: Support multiple entry loops? (We currently bail out of these in + // the IndVarSimplify pass) + if (auto *BB = L->getLoopPredecessor()) { + Value *Incoming = PN->getIncomingValueForBlock(BB); + const SCEV *IncomingS = SE->getSCEV(Incoming); + CheapExpansions[IncomingS] = Incoming; + } + Value *NewLHS = CheapExpansions[InvariantLHS]; + Value *NewRHS = CheapExpansions[InvariantRHS]; + + if (!NewLHS || !NewRHS) + // We could not find an existing value to replace either LHS or RHS. + // Generating new instructions has subtler tradeoffs, so avoid doing that + // for now. + return false; + + DEBUG(dbgs() << "INDVARS: Simplified comparison: " << *ICmp << '\n'); + ICmp->setPredicate(InvariantPredicate); + ICmp->setOperand(0, NewLHS); + ICmp->setOperand(1, NewRHS); + return true; } /// SimplifyIVUsers helper for eliminating useless @@ -210,23 +239,6 @@ void SimplifyIndvar::eliminateIVComparison(ICmpInst *ICmp, Value *IVOperand) { const SCEV *S = SE->getSCEVAtScope(ICmp->getOperand(IVOperIdx), ICmpLoop); const SCEV *X = SE->getSCEVAtScope(ICmp->getOperand(1 - IVOperIdx), ICmpLoop); - SmallDenseMap CheapExpansions; - CheapExpansions[S] = ICmp->getOperand(IVOperIdx); - CheapExpansions[X] = ICmp->getOperand(1 - IVOperIdx); - - // TODO: Support multiple entry loops? (We currently bail out of these in - // the IndVarSimplify pass) - auto *PN = dyn_cast(IVOperand); - if (PN) - if (auto *BB = L->getLoopPredecessor()) { - Value *Incoming = PN->getIncomingValueForBlock(BB); - const SCEV *IncomingS = SE->getSCEV(Incoming); - CheapExpansions[IncomingS] = Incoming; - } - - ICmpInst::Predicate NewPred; - Value *NewLHS = nullptr, *NewRHS = nullptr; - // If the condition is always true or always false, replace it with // a constant value. if (SE->isKnownPredicate(Pred, S, X)) { @@ -237,14 +249,8 @@ void SimplifyIndvar::eliminateIVComparison(ICmpInst *ICmp, Value *IVOperand) { ICmp->replaceAllUsesWith(ConstantInt::getFalse(ICmp->getContext())); DeadInsts.emplace_back(ICmp); DEBUG(dbgs() << "INDVARS: Eliminated comparison: " << *ICmp << '\n'); - } else if (PN && - isCheapLoopInvariantPredicate(Pred, S, X, L, CheapExpansions, - NewPred, NewLHS, NewRHS)) { - DEBUG(dbgs() << "INDVARS: Simplified comparison: " << *ICmp << '\n'); - ICmp->setPredicate(NewPred); - assert(NewLHS && NewRHS); - ICmp->setOperand(0, NewLHS); - ICmp->setOperand(1, NewRHS); + } else if (makeIVComparisonInvariant(ICmp, IVOperand)) { + // fallthrough to end of function } else if (ICmpInst::isSigned(OriginalPred) && SE->isKnownNonNegative(S) && SE->isKnownNonNegative(X)) { // If we were unable to make anything above, all we can is to canonicalize