Revert "[ScopDetect/Info] Look through PHIs that follow an error block"

This reverts commit
r312410 - [ScopDetect/Info] Look through PHIs that follow an error block

The commit caused generation of invalid IR due to accessing a parameter
that does not dominate the SCoP.

llvm-svn: 312663

polly/include/polly/ScopInfo.h

@@ -1697,7 +1697,6 @@ class Scop {
   friend class ScopBuilder;
 
   ScalarEvolution *SE;
-  DominatorTree *DT;
 
   /// The underlying Region.
   Region &R;
@@ -1926,9 +1925,12 @@ class Scop {
   static int getNextID(std::string ParentFunc);
 
   /// Scop constructor; invoked from ScopBuilder::buildScop.
-  Scop(Region &R, ScalarEvolution &SE, LoopInfo &LI, DominatorTree &DT,
+  Scop(Region &R, ScalarEvolution &SE, LoopInfo &LI,
        ScopDetection::DetectionContext &DC, OptimizationRemarkEmitter &ORE);
 
+  /// Return the LoopInfo used for this Scop.
+  LoopInfo *getLI() const { return Affinator.getLI(); }
+
   //@}
 
   /// Initialize this ScopBuilder.
@@ -2398,15 +2400,8 @@ class Scop {
   /// Remove the metadata stored for @p Access.
   void removeAccessData(MemoryAccess *Access);
 
-  /// Return the scalar evolution.
   ScalarEvolution *getSE() const;
 
-  /// Return the dominator tree.
-  DominatorTree *getDT() const { return DT; }
-
-  /// Return the LoopInfo used for this Scop.
-  LoopInfo *getLI() const { return Affinator.getLI(); }
-
   /// Get the count of parameters used in this Scop.
   ///
   /// @return The count of parameters used in this Scop.

polly/include/polly/Support/SCEVValidator.h

@@ -94,17 +94,6 @@ ParameterSetTy getParamsInAffineExpr(const llvm::Region *R, llvm::Loop *Scope,
 /// @returns The constant factor in @p M and the rest of @p M.
 std::pair<const llvm::SCEVConstant *, const llvm::SCEV *>
 extractConstantFactor(const llvm::SCEV *M, llvm::ScalarEvolution &SE);
-
-/// Try to look through PHI nodes, where some incoming edges come from error
-/// blocks.
-///
-/// In case a PHI node follows an error block we can assume that the incoming
-/// value can only come from the node that is not an error block. As a result,
-/// conditions that seemed non-affine before are now in fact affine.
-const llvm::SCEV *tryForwardThroughPHI(const llvm::SCEV *Expr, llvm::Region &R,
-                                       llvm::ScalarEvolution &SE,
-                                       llvm::LoopInfo &LI,
-                                       const llvm::DominatorTree &DT);
 } // namespace polly
 
 #endif

polly/lib/Analysis/ScopBuilder.cpp

@@ -1181,7 +1181,7 @@ static inline BasicBlock *getRegionNodeBasicBlock(RegionNode *RN) {
 
 void ScopBuilder::buildScop(Region &R, AssumptionCache &AC,
                             OptimizationRemarkEmitter &ORE) {
-  scop.reset(new Scop(R, SE, LI, DT, *SD.getDetectionContext(&R), ORE));
+  scop.reset(new Scop(R, SE, LI, *SD.getDetectionContext(&R), ORE));
 
   buildStmts(R);
   buildAccessFunctions();

polly/lib/Analysis/ScopDetection.cpp

@@ -607,9 +607,6 @@ bool ScopDetection::isValidBranch(BasicBlock &BB, BranchInst *BI,
   const SCEV *LHS = SE.getSCEVAtScope(ICmp->getOperand(0), L);
   const SCEV *RHS = SE.getSCEVAtScope(ICmp->getOperand(1), L);
 
-  LHS = tryForwardThroughPHI(LHS, Context.CurRegion, SE, LI, DT);
-  RHS = tryForwardThroughPHI(RHS, Context.CurRegion, SE, LI, DT);
-
   // If unsigned operations are not allowed try to approximate the region.
   if (ICmp->isUnsigned() && !PollyAllowUnsignedOperations)
     return !IsLoopBranch && AllowNonAffineSubRegions &&

polly/lib/Analysis/ScopInfo.cpp

@@ -1451,10 +1451,11 @@ getPwAff(Scop &S, BasicBlock *BB,
 /// This will fill @p ConditionSets with the conditions under which control
 /// will be moved from @p SI to its successors. Hence, @p ConditionSets will
 /// have as many elements as @p SI has successors.
-bool buildConditionSets(Scop &S, BasicBlock *BB, SwitchInst *SI, Loop *L,
-                        __isl_keep isl_set *Domain,
-                        DenseMap<BasicBlock *, isl::set> &InvalidDomainMap,
-                        SmallVectorImpl<__isl_give isl_set *> &ConditionSets) {
+static bool
+buildConditionSets(Scop &S, BasicBlock *BB, SwitchInst *SI, Loop *L,
+                   __isl_keep isl_set *Domain,
+                   DenseMap<BasicBlock *, isl::set> &InvalidDomainMap,
+                   SmallVectorImpl<__isl_give isl_set *> &ConditionSets) {
   Value *Condition = getConditionFromTerminator(SI);
   assert(Condition && "No condition for switch");
 
@@ -1496,7 +1497,7 @@ bool buildConditionSets(Scop &S, BasicBlock *BB, SwitchInst *SI, Loop *L,
 /// @param IsStrictUpperBound holds information on the predicate relation
 /// between TestVal and UpperBound, i.e,
 /// TestVal < UpperBound  OR  TestVal <= UpperBound
-__isl_give isl_set *
+static __isl_give isl_set *
 buildUnsignedConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
                            __isl_keep isl_set *Domain, const SCEV *SCEV_TestVal,
                            const SCEV *SCEV_UpperBound,
@@ -1536,10 +1537,11 @@ buildUnsignedConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
 /// have as many elements as @p TI has successors. If @p TI is nullptr the
 /// context under which @p Condition is true/false will be returned as the
 /// new elements of @p ConditionSets.
-bool buildConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
-                        TerminatorInst *TI, Loop *L, __isl_keep isl_set *Domain,
-                        DenseMap<BasicBlock *, isl::set> &InvalidDomainMap,
-                        SmallVectorImpl<__isl_give isl_set *> &ConditionSets) {
+static bool
+buildConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
+                   TerminatorInst *TI, Loop *L, __isl_keep isl_set *Domain,
+                   DenseMap<BasicBlock *, isl::set> &InvalidDomainMap,
+                   SmallVectorImpl<__isl_give isl_set *> &ConditionSets) {
   isl_set *ConsequenceCondSet = nullptr;
   if (auto *CCond = dyn_cast<ConstantInt>(Condition)) {
     if (CCond->isZero())
@@ -1575,10 +1577,6 @@ bool buildConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
            "Condition of exiting branch was neither constant nor ICmp!");
 
     ScalarEvolution &SE = *S.getSE();
-    LoopInfo &LI = *S.getLI();
-    DominatorTree &DT = *S.getDT();
-    Region &R = S.getRegion();
-
     isl_pw_aff *LHS, *RHS;
     // For unsigned comparisons we assumed the signed bit of neither operand
     // to be set. The comparison is equal to a signed comparison under this
@@ -1587,9 +1585,6 @@ bool buildConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
     const SCEV *LeftOperand = SE.getSCEVAtScope(ICond->getOperand(0), L),
                *RightOperand = SE.getSCEVAtScope(ICond->getOperand(1), L);
 
-    LeftOperand = tryForwardThroughPHI(LeftOperand, R, SE, LI, DT);
-    RightOperand = tryForwardThroughPHI(RightOperand, R, SE, LI, DT);
-
     switch (ICond->getPredicate()) {
     case ICmpInst::ICMP_ULT:
       ConsequenceCondSet =
@@ -1658,10 +1653,11 @@ bool buildConditionSets(Scop &S, BasicBlock *BB, Value *Condition,
 /// This will fill @p ConditionSets with the conditions under which control
 /// will be moved from @p TI to its successors. Hence, @p ConditionSets will
 /// have as many elements as @p TI has successors.
-bool buildConditionSets(Scop &S, BasicBlock *BB, TerminatorInst *TI, Loop *L,
-                        __isl_keep isl_set *Domain,
-                        DenseMap<BasicBlock *, isl::set> &InvalidDomainMap,
-                        SmallVectorImpl<__isl_give isl_set *> &ConditionSets) {
+static bool
+buildConditionSets(Scop &S, BasicBlock *BB, TerminatorInst *TI, Loop *L,
+                   __isl_keep isl_set *Domain,
+                   DenseMap<BasicBlock *, isl::set> &InvalidDomainMap,
+                   SmallVectorImpl<__isl_give isl_set *> &ConditionSets) {
   if (SwitchInst *SI = dyn_cast<SwitchInst>(TI))
     return buildConditionSets(S, BB, SI, L, Domain, InvalidDomainMap,
                               ConditionSets);
@@ -3361,9 +3357,8 @@ int Scop::getNextID(std::string ParentFunc) {
 }
 
 Scop::Scop(Region &R, ScalarEvolution &ScalarEvolution, LoopInfo &LI,
-           DominatorTree &DT, ScopDetection::DetectionContext &DC,
-           OptimizationRemarkEmitter &ORE)
-    : SE(&ScalarEvolution), DT(&DT), R(R), name(R.getNameStr()),
+           ScopDetection::DetectionContext &DC, OptimizationRemarkEmitter &ORE)
+    : SE(&ScalarEvolution), R(R), name(R.getNameStr()),
       HasSingleExitEdge(R.getExitingBlock()), DC(DC), ORE(ORE),
       IslCtx(isl_ctx_alloc(), isl_ctx_free), Affinator(this, LI),
       ID(getNextID((*R.getEntry()->getParent()).getName().str())) {

polly/lib/Support/SCEVValidator.cpp

@@ -735,30 +735,4 @@ extractConstantFactor(const SCEV *S, ScalarEvolution &SE) {
 
   return std::make_pair(ConstPart, SE.getMulExpr(LeftOvers));
 }
-
-const SCEV *tryForwardThroughPHI(const SCEV *Expr, Region &R,
-                                 ScalarEvolution &SE, LoopInfo &LI,
-                                 const DominatorTree &DT) {
-  if (auto *Unknown = dyn_cast<SCEVUnknown>(Expr)) {
-    Value *V = Unknown->getValue();
-    auto *PHI = dyn_cast<PHINode>(V);
-    if (!PHI)
-      return Expr;
-
-    Value *Final = nullptr;
-
-    for (unsigned i = 0; i < PHI->getNumIncomingValues(); i++) {
-      if (isErrorBlock(*PHI->getIncomingBlock(i), R, LI, DT))
-        continue;
-      if (Final)
-        return Expr;
-      Final = PHI->getIncomingValue(i);
-    }
-
-    if (Final)
-      return SE.getSCEV(Final);
-  }
-  return Expr;
-}
-
 } // namespace polly