[polly] r249273 - [FIX] Approximate non-affine loops correctly

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 4 07:54:27 PDT 2015


Author: jdoerfert
Date: Sun Oct  4 09:54:27 2015
New Revision: 249273

URL: http://llvm.org/viewvc/llvm-project?rev=249273&view=rev
Log:
[FIX] Approximate non-affine loops correctly

  Before isValidCFG() could hide the fact that a loop is non-affine by
  over-approximation. This is problematic if a subregion of the loop contains
  an exit/latch block and is over-approximated. Now we do not over-approximate
  in the isValidCFG function if we check loop control.  If such control is
  non-affine the whole loop is over-approximated, not only a subregion.


Added:
    polly/trunk/test/ScopDetect/invalid-latch-conditions.ll
Modified:
    polly/trunk/include/polly/ScopDetection.h
    polly/trunk/lib/Analysis/ScopDetection.cpp

Modified: polly/trunk/include/polly/ScopDetection.h
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopDetection.h?rev=249273&r1=249272&r2=249273&view=diff
==============================================================================
--- polly/trunk/include/polly/ScopDetection.h (original)
+++ polly/trunk/include/polly/ScopDetection.h Sun Oct  4 09:54:27 2015
@@ -281,33 +281,37 @@ private:
 
   /// @brief Check if the switch @p SI with condition @p Condition is valid.
   ///
-  /// @param BB        The block to check.
-  /// @param SI        The switch to check.
-  /// @param Condition The switch condition.
-  /// @param Context   The context of scop detection.
+  /// @param BB           The block to check.
+  /// @param SI           The switch to check.
+  /// @param Condition    The switch condition.
+  /// @param IsLoopBranch Flag to indicate the branch is a loop exit/latch.
+  /// @param Context      The context of scop detection.
   ///
   /// @return True if the branch @p BI is valid.
   bool isValidSwitch(BasicBlock &BB, SwitchInst *SI, Value *Condition,
-                     DetectionContext &Context) const;
+                     bool IsLoopBranch, DetectionContext &Context) const;
 
   /// @brief Check if the branch @p BI with condition @p Condition is valid.
   ///
-  /// @param BB        The block to check.
-  /// @param BI        The branch to check.
-  /// @param Condition The branch condition.
-  /// @param Context   The context of scop detection.
+  /// @param BB           The block to check.
+  /// @param BI           The branch to check.
+  /// @param Condition    The branch condition.
+  /// @param IsLoopBranch Flag to indicate the branch is a loop exit/latch.
+  /// @param Context      The context of scop detection.
   ///
   /// @return True if the branch @p BI is valid.
   bool isValidBranch(BasicBlock &BB, BranchInst *BI, Value *Condition,
-                     DetectionContext &Context) const;
+                     bool IsLoopBranch, DetectionContext &Context) const;
 
   /// @brief Check if the control flow in a basic block is valid.
   ///
-  /// @param BB The BB to check the control flow.
-  /// @param Context The context of scop detection.
+  /// @param BB           The BB to check the control flow.
+  /// @param IsLoopBranch Flag to indicate the branch is a loop exit/latch.
+  /// @param Context      The context of scop detection.
   ///
   /// @return True if the BB contains only valid control flow.
-  bool isValidCFG(BasicBlock &BB, DetectionContext &Context) const;
+  bool isValidCFG(BasicBlock &BB, bool IsLoopBranch,
+                  DetectionContext &Context) const;
 
   /// @brief Is a loop valid with respect to a given region.
   ///

Modified: polly/trunk/lib/Analysis/ScopDetection.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=249273&r1=249272&r2=249273&view=diff
==============================================================================
--- polly/trunk/lib/Analysis/ScopDetection.cpp (original)
+++ polly/trunk/lib/Analysis/ScopDetection.cpp Sun Oct  4 09:54:27 2015
@@ -300,71 +300,79 @@ bool ScopDetection::addOverApproximatedR
 }
 
 bool ScopDetection::isValidSwitch(BasicBlock &BB, SwitchInst *SI,
-                                  Value *Condition,
+                                  Value *Condition, bool IsLoopBranch,
                                   DetectionContext &Context) const {
   Region &CurRegion = Context.CurRegion;
 
   Loop *L = LI->getLoopFor(&BB);
   const SCEV *ConditionSCEV = SE->getSCEVAtScope(Condition, L);
 
-  if (!isAffineExpr(&CurRegion, ConditionSCEV, *SE))
-    if (!AllowNonAffineSubRegions ||
-        !addOverApproximatedRegion(RI->getRegionFor(&BB), Context))
-      return invalid<ReportNonAffBranch>(Context, /*Assert=*/true, &BB,
-                                         ConditionSCEV, ConditionSCEV, SI);
+  if (isAffineExpr(&CurRegion, ConditionSCEV, *SE))
+    return true;
 
-  return true;
+  if (!IsLoopBranch && AllowNonAffineSubRegions &&
+      addOverApproximatedRegion(RI->getRegionFor(&BB), Context))
+    return true;
+
+  if (IsLoopBranch)
+    return false;
+
+  return invalid<ReportNonAffBranch>(Context, /*Assert=*/true, &BB,
+                                     ConditionSCEV, ConditionSCEV, SI);
 }
 
 bool ScopDetection::isValidBranch(BasicBlock &BB, BranchInst *BI,
-                                  Value *Condition,
+                                  Value *Condition, bool IsLoopBranch,
                                   DetectionContext &Context) const {
   Region &CurRegion = Context.CurRegion;
 
   // Non constant conditions of branches need to be ICmpInst.
   if (!isa<ICmpInst>(Condition)) {
-    if (!AllowNonAffineSubRegions ||
-        !addOverApproximatedRegion(RI->getRegionFor(&BB), Context))
-      return invalid<ReportInvalidCond>(Context, /*Assert=*/true, BI, &BB);
+    if (!IsLoopBranch && AllowNonAffineSubRegions &&
+        addOverApproximatedRegion(RI->getRegionFor(&BB), Context))
+      return true;
+    return invalid<ReportInvalidCond>(Context, /*Assert=*/true, BI, &BB);
   }
 
-  if (ICmpInst *ICmp = dyn_cast<ICmpInst>(Condition)) {
-    // Unsigned comparisons are not allowed. They trigger overflow problems
-    // in the code generation.
-    //
-    // TODO: This is not sufficient and just hides bugs. However it does pretty
-    // well.
-    if (ICmp->isUnsigned() && !AllowUnsigned)
-      return invalid<ReportUnsignedCond>(Context, /*Assert=*/true, BI, &BB);
-
-    // Are both operands of the ICmp affine?
-    if (isa<UndefValue>(ICmp->getOperand(0)) ||
-        isa<UndefValue>(ICmp->getOperand(1)))
-      return invalid<ReportUndefOperand>(Context, /*Assert=*/true, &BB, ICmp);
-
-    // TODO: FIXME: IslExprBuilder is not capable of producing valid code
-    //              for arbitrary pointer expressions at the moment. Until
-    //              this is fixed we disallow pointer expressions completely.
-    if (ICmp->getOperand(0)->getType()->isPointerTy())
-      return false;
+  ICmpInst *ICmp = cast<ICmpInst>(Condition);
+  // Unsigned comparisons are not allowed. They trigger overflow problems
+  // in the code generation.
+  //
+  // TODO: This is not sufficient and just hides bugs. However it does pretty
+  //       well.
+  if (ICmp->isUnsigned() && !AllowUnsigned)
+    return invalid<ReportUnsignedCond>(Context, /*Assert=*/true, BI, &BB);
+
+  // Are both operands of the ICmp affine?
+  if (isa<UndefValue>(ICmp->getOperand(0)) ||
+      isa<UndefValue>(ICmp->getOperand(1)))
+    return invalid<ReportUndefOperand>(Context, /*Assert=*/true, &BB, ICmp);
+
+  // TODO: FIXME: IslExprBuilder is not capable of producing valid code
+  //              for arbitrary pointer expressions at the moment. Until
+  //              this is fixed we disallow pointer expressions completely.
+  if (ICmp->getOperand(0)->getType()->isPointerTy())
+    return false;
 
-    Loop *L = LI->getLoopFor(ICmp->getParent());
-    const SCEV *LHS = SE->getSCEVAtScope(ICmp->getOperand(0), L);
-    const SCEV *RHS = SE->getSCEVAtScope(ICmp->getOperand(1), L);
-
-    if (!isAffineExpr(&CurRegion, LHS, *SE) ||
-        !isAffineExpr(&CurRegion, RHS, *SE)) {
-      if (!AllowNonAffineSubRegions ||
-          !addOverApproximatedRegion(RI->getRegionFor(&BB), Context))
-        return invalid<ReportNonAffBranch>(Context, /*Assert=*/true, &BB, LHS,
-                                           RHS, ICmp);
-    }
-  }
+  Loop *L = LI->getLoopFor(ICmp->getParent());
+  const SCEV *LHS = SE->getSCEVAtScope(ICmp->getOperand(0), L);
+  const SCEV *RHS = SE->getSCEVAtScope(ICmp->getOperand(1), L);
 
-  return true;
+  if (isAffineExpr(&CurRegion, LHS, *SE) && isAffineExpr(&CurRegion, RHS, *SE))
+    return true;
+
+  if (!IsLoopBranch && AllowNonAffineSubRegions &&
+      addOverApproximatedRegion(RI->getRegionFor(&BB), Context))
+    return true;
+
+  if (IsLoopBranch)
+    return false;
+
+  return invalid<ReportNonAffBranch>(Context, /*Assert=*/true, &BB, LHS, RHS,
+                                     ICmp);
 }
 
-bool ScopDetection::isValidCFG(BasicBlock &BB,
+bool ScopDetection::isValidCFG(BasicBlock &BB, bool IsLoopBranch,
                                DetectionContext &Context) const {
   Region &CurRegion = Context.CurRegion;
 
@@ -388,12 +396,12 @@ bool ScopDetection::isValidCFG(BasicBloc
     return true;
 
   if (BranchInst *BI = dyn_cast<BranchInst>(TI))
-    return isValidBranch(BB, BI, Condition, Context);
+    return isValidBranch(BB, BI, Condition, IsLoopBranch, Context);
 
   SwitchInst *SI = dyn_cast<SwitchInst>(TI);
   assert(SI && "Terminator was neither branch nor switch");
 
-  return isValidSwitch(BB, SI, Condition, Context);
+  return isValidSwitch(BB, SI, Condition, IsLoopBranch, Context);
 }
 
 bool ScopDetection::isValidCallInst(CallInst &CI) {
@@ -735,7 +743,7 @@ bool ScopDetection::canUseISLTripCount(L
   L->getLoopLatches(LoopControlBlocks);
   L->getExitingBlocks(LoopControlBlocks);
   for (BasicBlock *ControlBB : LoopControlBlocks) {
-    if (!isValidCFG(*ControlBB, Context))
+    if (!isValidCFG(*ControlBB, true, Context))
       return false;
   }
 
@@ -751,9 +759,11 @@ bool ScopDetection::isValidLoop(Loop *L,
 
   if (AllowNonAffineSubLoops && AllowNonAffineSubRegions) {
     Region *R = RI->getRegionFor(L->getHeader());
-    if (R->contains(L))
-      if (addOverApproximatedRegion(R, Context))
-        return true;
+    while (R != &Context.CurRegion && !R->contains(L))
+      R = R->getParent();
+
+    if (addOverApproximatedRegion(R, Context))
+      return true;
   }
 
   const SCEV *LoopCount = SE->getBackedgeTakenCount(L);
@@ -948,7 +958,7 @@ bool ScopDetection::allBlocksValid(Detec
     if (isErrorBlock(*BB))
       continue;
 
-    if (!isValidCFG(*BB, Context) && !KeepGoing)
+    if (!isValidCFG(*BB, false, Context) && !KeepGoing)
       return false;
     for (BasicBlock::iterator I = BB->begin(), E = --BB->end(); I != E; ++I)
       if (!isValidInstruction(*I, Context) && !KeepGoing)

Added: polly/trunk/test/ScopDetect/invalid-latch-conditions.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopDetect/invalid-latch-conditions.ll?rev=249273&view=auto
==============================================================================
--- polly/trunk/test/ScopDetect/invalid-latch-conditions.ll (added)
+++ polly/trunk/test/ScopDetect/invalid-latch-conditions.ll Sun Oct  4 09:54:27 2015
@@ -0,0 +1,39 @@
+; RUN: opt %loadPolly -polly-detect -analyze < %s | FileCheck %s
+; RUN: opt %loadPolly -polly-allow-nonaffine-loops -polly-detect -analyze < %s | FileCheck %s --check-prefix=NALOOPS
+
+; The latch conditions of the outer loop are not affine, thus the loop cannot
+; handled by the domain generation and needs to be overapproximated.
+
+; CHECK-NOT: Valid
+; NALOOPS:   Valid Region for Scop: for.body.6 => for.end.45
+
+; ModuleID = '/home/johannes/Downloads/bug.ll'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; Function Attrs: nounwind uwtable
+define void @kernel_reg_detect([6 x i32]* %path) #0 {
+entry:
+  br label %for.body.6
+
+for.body.6:                                       ; preds = %for.inc.43, %for.body.6, %entry
+  %indvars.iv9 = phi i64 [ %indvars.iv.next10, %for.body.6 ], [ 0, %for.inc.43 ], [ 0, %entry ]
+  %indvars.iv.next10 = add nuw nsw i64 %indvars.iv9, 1
+  %lftr.wideiv = trunc i64 %indvars.iv.next10 to i32
+  %exitcond = icmp ne i32 %lftr.wideiv, 6
+  br i1 %exitcond, label %for.body.6, label %for.inc.40
+
+for.inc.40:                                       ; preds = %for.inc.40, %for.body.6
+  %arrayidx28 = getelementptr inbounds [6 x i32], [6 x i32]* %path, i64 0, i64 0
+  %tmp = load i32, i32* %arrayidx28, align 4
+  %arrayidx36 = getelementptr inbounds [6 x i32], [6 x i32]* %path, i64 0, i64 0
+  store i32 0, i32* %arrayidx36, align 4
+  %exitcond22 = icmp ne i64 0, 6
+  br i1 %exitcond22, label %for.inc.40, label %for.inc.43
+
+for.inc.43:                                       ; preds = %for.inc.40
+  %exitcond23 = icmp ne i32 0, 10000
+  br i1 %exitcond23, label %for.body.6, label %for.end.45
+
+for.end.45:                                       ; preds = %for.inc.43
+  ret void
+}




More information about the llvm-commits mailing list