[llvm] r317116 - Revert 317016 and 317048

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 1 12:49:20 PDT 2017


Author: reames
Date: Wed Nov  1 12:49:20 2017
New Revision: 317116

URL: http://llvm.org/viewvc/llvm-project?rev=317116&view=rev
Log:
Revert 317016 and 317048

The former appears to have introduced a miscompile in a stage2 clang build.  Revert so I can investigate offline.


Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp?rev=317116&r1=317115&r2=317116&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp Wed Nov  1 12:49:20 2017
@@ -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<const SCEV*, Value*> &FreeExpansions,
-           ICmpInst::Predicate &InvariantPred,
-           Value *&LHSV, Value *& RHSV);
   };
 }
 
@@ -171,24 +162,62 @@ Value *SimplifyIndvar::foldIVUser(Instru
   return IVSrc;
 }
 
-bool SimplifyIndvar::isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred,
-           const SCEV *LHS, const SCEV *RHS, const Loop *L,
-           const SmallDenseMap<const SCEV*, Value*> &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<PHINode>(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<const SCEV*, Value*> 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::eliminateIVComparis
   const SCEV *S = SE->getSCEVAtScope(ICmp->getOperand(IVOperIdx), ICmpLoop);
   const SCEV *X = SE->getSCEVAtScope(ICmp->getOperand(1 - IVOperIdx), ICmpLoop);
 
-  SmallDenseMap<const SCEV*, Value*> 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<PHINode>(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::eliminateIVComparis
     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




More information about the llvm-commits mailing list