Allow PHI nodes in the region exit block

  While we do not need to model PHI nodes in the region exit (as it is not part
  of the SCoP), we need to prepare for the case that the exit block is split in
  code generation to create a single exiting block. If this will happen, hence
  if the region did not have a single exiting block before, we will model the
  operands of the PHI nodes as escaping scalars in the SCoP.

Differential Revision: http://reviews.llvm.org/D12051

llvm-svn: 247078

polly/include/polly/CodeGen/BlockGenerators.h

@@ -386,7 +386,9 @@ class BlockGenerator {
   /// @param Inst      The current instruction we check.
   /// @param InstCopy  The copy of the instruction @p Inst in the optimized
   ///                  SCoP.
-  void handleOutsideUsers(const Region &R, Instruction *Inst, Value *InstCopy);
+  /// @param Address   If given it is used as the escape address for @p Inst.
+  void handleOutsideUsers(const Region &R, Instruction *Inst, Value *InstCopy,
+                          AllocaInst *Address = nullptr);
 
   /// @brief Initialize the memory of demoted scalars.
   ///

polly/include/polly/ScopDetection.h

@@ -223,13 +223,6 @@ class ScopDetection : public FunctionPass {
   /// @return True if all blocks in R are valid, false otherwise.
   bool allBlocksValid(DetectionContext &Context) const;
 
-  /// @brief Check the exit block of a region is valid.
-  ///
-  /// @param Context The context of scop detection.
-  ///
-  /// @return True if the exit of R is valid, false otherwise.
-  bool isValidExit(DetectionContext &Context) const;
-
   /// @brief Check if a region is a Scop.
   ///
   /// @param Context The context of scop detection.

polly/include/polly/ScopInfo.h

@@ -799,6 +799,9 @@ class Scop {
   /// Flag to indicate that the scheduler actually optimized the SCoP.
   bool IsOptimized;
 
+  /// @brief True if the underlying region has a single exiting block.
+  bool HasSingleExitEdge;
+
   /// Max loop depth.
   unsigned MaxLoopDepth;
 
@@ -1192,6 +1195,9 @@ class Scop {
   /// @brief Align the parameters in the statement to the scop context
   void realignParams();
 
+  /// @brief Return true if the underlying region has a single exiting block.
+  bool hasSingleExitEdge() const { return HasSingleExitEdge; }
+
   /// @brief Print the static control part.
   ///
   /// @param OS The output stream the static control part is printed to.

polly/include/polly/TempScopInfo.h

@@ -256,8 +256,9 @@ class TempScopInfo : public RegionPass {
   /// @param R                  The SCoP region
   /// @param Functions          The access functions of the current BB
   /// @param NonAffineSubRegion The non affine sub-region @p PHI is in.
+  /// @param IsExitBlock        Flag to indicate that @p PHI is in the exit BB.
   void buildPHIAccesses(PHINode *PHI, Region &R, AccFuncSetType &Functions,
-                        Region *NonAffineSubRegion);
+                        Region *NonAffineSubRegion, bool IsExitBlock = false);
 
   /// @brief Build the access functions for the subregion @p SR.
   ///
@@ -270,8 +271,10 @@ class TempScopInfo : public RegionPass {
   /// @param R                  The SCoP region.
   /// @param BB                 A basic block in @p R.
   /// @param NonAffineSubRegion The non affine sub-region @p BB is in.
+  /// @param IsExitBlock        Flag to indicate that @p BB is in the exit BB.
   void buildAccessFunctions(Region &R, BasicBlock &BB,
-                            Region *NonAffineSubRegion = nullptr);
+                            Region *NonAffineSubRegion = nullptr,
+                            bool IsExitBlock = false);
 
 public:
   static char ID;

polly/lib/Analysis/ScopDetection.cpp

@@ -802,8 +802,7 @@ Region *ScopDetection::expandRegion(Region &R) {
     DEBUG(dbgs() << "\t\tTrying " << ExpandedRegion->getNameStr() << "\n");
     // Only expand when we did not collect errors.
 
-    // Check the exit first (cheap)
-    if (isValidExit(Context) && !Context.Log.hasErrors()) {
+    if (!Context.Log.hasErrors()) {
       // If the exit is valid check all blocks
       //  - if true, a valid region was found => store it + keep expanding
       //  - if false, .tbd. => stop  (should this really end the loop?)
@@ -945,18 +944,6 @@ bool ScopDetection::allBlocksValid(DetectionContext &Context) const {
   return true;
 }
 
-bool ScopDetection::isValidExit(DetectionContext &Context) const {
-
-  // PHI nodes are not allowed in the exit basic block.
-  if (BasicBlock *Exit = Context.CurRegion.getExit()) {
-    BasicBlock::iterator I = Exit->begin();
-    if (I != Exit->end() && isa<PHINode>(*I))
-      return invalid<ReportPHIinExit>(Context, /*Assert=*/true, I);
-  }
-
-  return true;
-}
-
 bool ScopDetection::isValidRegion(DetectionContext &Context) const {
   Region &CurRegion = Context.CurRegion;
 
@@ -1002,9 +989,6 @@ bool ScopDetection::isValidRegion(DetectionContext &Context) const {
   if (!DetectUnprofitable && !hasMoreThanOneLoop(&CurRegion))
     invalid<ReportUnprofitable>(Context, /*Assert=*/true, &CurRegion);
 
-  if (!isValidExit(Context))
-    return false;
-
   if (!allBlocksValid(Context))
     return false;
 

polly/lib/Analysis/ScopInfo.cpp

@@ -1819,7 +1819,8 @@ static unsigned getMaxLoopDepthInRegion(const Region &R, LoopInfo &LI,
 Scop::Scop(Region &R, ScalarEvolution &ScalarEvolution, DominatorTree &DT,
            isl_ctx *Context, unsigned MaxLoopDepth)
     : DT(DT), SE(&ScalarEvolution), R(R), IsOptimized(false),
-      MaxLoopDepth(MaxLoopDepth), IslCtx(Context), Affinator(this) {}
+      HasSingleExitEdge(R.getExitingBlock()), MaxLoopDepth(MaxLoopDepth),
+      IslCtx(Context), Affinator(this) {}
 
 void Scop::initFromTempScop(TempScop &TempScop, LoopInfo &LI, ScopDetection &SD,
                             AliasAnalysis &AA) {

polly/lib/Analysis/TempScopInfo.cpp

@@ -92,8 +92,18 @@ void TempScop::printDetail(raw_ostream &OS, ScalarEvolution *SE, LoopInfo *LI,
 
 void TempScopInfo::buildPHIAccesses(PHINode *PHI, Region &R,
                                     AccFuncSetType &Functions,
-                                    Region *NonAffineSubRegion) {
-  if (canSynthesize(PHI, LI, SE, &R))
+                                    Region *NonAffineSubRegion,
+                                    bool IsExitBlock) {
+
+  // PHI nodes that are in the exit block of the region, hence if IsExitBlock is
+  // true, are not modeled as ordinary PHI nodes as they are not part of the
+  // region. However, we model the operands in the predecessor blocks that are
+  // part of the region as regular scalar accesses.
+
+  // If we can synthesize a PHI we can skip it, however only if it is in
+  // the region. If it is not it can only be in the exit block of the region.
+  // In this case we model the operands but not the PHI itself.
+  if (!IsExitBlock && canSynthesize(PHI, LI, SE, &R))
     return;
 
   // PHI nodes are modeled as if they had been demoted prior to the SCoP
@@ -133,13 +143,13 @@ void TempScopInfo::buildPHIAccesses(PHINode *PHI, Region &R,
     OpI = OpBB->getTerminator();
 
     IRAccess ScalarAccess(IRAccess::MUST_WRITE, PHI, ZeroOffset, 1, true, Op,
-                          /* IsPHI */ true);
+                          /* IsPHI */ !IsExitBlock);
     AccFuncMap[OpBB].push_back(std::make_pair(ScalarAccess, OpI));
   }
 
   if (!OnlyNonAffineSubRegionOperands) {
     IRAccess ScalarAccess(IRAccess::READ, PHI, ZeroOffset, 1, true, PHI,
-                          /* IsPHI */ true);
+                          /* IsPHI */ !IsExitBlock);
     Functions.push_back(std::make_pair(ScalarAccess, PHI));
   }
 }
@@ -297,7 +307,8 @@ void TempScopInfo::buildAccessFunctions(Region &R, Region &SR) {
 }
 
 void TempScopInfo::buildAccessFunctions(Region &R, BasicBlock &BB,
-                                        Region *NonAffineSubRegion) {
+                                        Region *NonAffineSubRegion,
+                                        bool IsExitBlock) {
   AccFuncSetType Functions;
   Loop *L = LI->getLoopFor(&BB);
 
@@ -306,16 +317,22 @@ void TempScopInfo::buildAccessFunctions(Region &R, BasicBlock &BB,
 
   for (BasicBlock::iterator I = BB.begin(), E = --BB.end(); I != E; ++I) {
     Instruction *Inst = I;
+
+    PHINode *PHI = dyn_cast<PHINode>(Inst);
+    if (PHI)
+      buildPHIAccesses(PHI, R, Functions, NonAffineSubRegion, IsExitBlock);
+
+    // For the exit block we stop modeling after the last PHI node.
+    if (!PHI && IsExitBlock)
+      break;
+
     if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst))
       Functions.push_back(
           std::make_pair(buildIRAccess(Inst, L, &R, BoxedLoops), Inst));
 
     if (isIgnoredIntrinsic(Inst))
       continue;
 
-    if (PHINode *PHI = dyn_cast<PHINode>(Inst))
-      buildPHIAccesses(PHI, R, Functions, NonAffineSubRegion);
-
     if (buildScalarDependences(Inst, &R, NonAffineSubRegion)) {
       // If the Instruction is used outside the statement, we need to build the
       // write access.
@@ -339,6 +356,16 @@ TempScop *TempScopInfo::buildTempScop(Region &R) {
 
   buildAccessFunctions(R, R);
 
+  // In case the region does not have an exiting block we will later (during
+  // code generation) split the exit block. This will move potential PHI nodes
+  // from the current exit block into the new region exiting block. Hence, PHI
+  // nodes that are at this point not part of the region will be.
+  // To handle these PHI nodes later we will now model their operands as scalar
+  // accesses. Note that we do not model anything in the exit block if we have
+  // an exiting block in the region, as there will not be any splitting later.
+  if (!R.getExitingBlock())
+    buildAccessFunctions(R, *R.getExit(), nullptr, /* IsExitBlock */ true);
+
   return TScop;
 }
 

polly/lib/CodeGen/BlockGenerators.cpp

@@ -365,7 +365,7 @@ Value *BlockGenerator::getOrCreatePHIAlloca(Value *ScalarBase) {
 }
 
 void BlockGenerator::handleOutsideUsers(const Region &R, Instruction *Inst,
-                                        Value *InstCopy) {
+                                        Value *InstCopy, AllocaInst *Address) {
   // If there are escape users we get the alloca for this instruction and put it
   // in the EscapeMap for later finalization. Lastly, if the instruction was
   // copied multiple times we already did this and can exit.
@@ -391,7 +391,8 @@ void BlockGenerator::handleOutsideUsers(const Region &R, Instruction *Inst,
     return;
 
   // Get or create an escape alloca for this instruction.
-  auto *ScalarAddr = cast<AllocaInst>(getOrCreateScalarAlloca(Inst));
+  auto *ScalarAddr =
+      Address ? Address : cast<AllocaInst>(getOrCreateScalarAlloca(Inst));
 
   // Remember that this instruction has escape uses and the escape alloca.
   EscapeMap[Inst] = std::make_pair(ScalarAddr, std::move(EscapeUsers));
@@ -516,6 +517,11 @@ void BlockGenerator::createScalarInitialization(Scop &S) {
     if (Inst && R.contains(Inst))
       continue;
 
+    // PHI nodes that are not marked as such in their SAI object are exit PHI
+    // nodes we model as common scalars but do not need to initialize.
+    if (Inst && isa<PHINode>(Inst))
+      continue;
+
     ValueMapT EmptyMap;
     Builder.CreateStore(Array->getBasePtr(),
                         getOrCreateScalarAlloca(Array->getBasePtr()));
@@ -567,6 +573,23 @@ void BlockGenerator::createScalarFinalization(Region &R) {
 }
 
 void BlockGenerator::finalizeSCoP(Scop &S) {
+
+  // Handle PHI nodes that were in the original exit and are now
+  // moved into the region exiting block.
+  if (!S.hasSingleExitEdge()) {
+    for (Instruction &I : *S.getRegion().getExitingBlock()) {
+      PHINode *PHI = dyn_cast<PHINode>(&I);
+      if (!PHI)
+        break;
+
+      assert(PHI->getNumUses() == 1);
+      assert(ScalarMap.count(PHI->user_back()));
+
+      handleOutsideUsers(S.getRegion(), PHI, nullptr,
+                         ScalarMap[PHI->user_back()]);
+    }
+  }
+
   createScalarInitialization(S);
   createScalarFinalization(S.getRegion());
 }

polly/test/Isl/CodeGen/phi_in_exit_early_lnt_failure_1.ll

@@ -0,0 +1,42 @@
+; RUN: opt %loadPolly -polly-detect-unprofitable -polly-codegen -polly-no-early-exit -S < %s | FileCheck %s
+;
+; This caused an lnt crash at some point, just verify it will run through.
+;
+; CHECK-LABEL: polly.merge_new_and_old:
+; CHECK-NEXT:    br label %for.body.6
+;
+; CHECK-LABEL: for.body.6:
+; CHECK-NEXT:    %i.14 = phi i32 [ undef, %for.body.6 ], [ 0, %polly.merge_new_and_old ]
+;
+@recd = external hidden global [255 x i32], align 16
+
+define void @rsdec_204(i8* %data_in) {
+entry:
+  br i1 undef, label %if.then, label %for.body
+
+if.then:                                          ; preds = %entry
+  unreachable
+
+for.body:                                         ; preds = %for.body, %entry
+  %i.05 = phi i32 [ %inc, %for.body ], [ 0, %entry ]
+  %arrayidx = getelementptr inbounds i8, i8* %data_in, i64 0
+  %0 = load i8, i8* %arrayidx, align 1
+  %conv = zext i8 %0 to i32
+  %arrayidx2 = getelementptr inbounds [255 x i32], [255 x i32]* @recd, i64 0, i64 0
+  store i32 %conv, i32* %arrayidx2, align 4
+  %inc = add nuw nsw i32 %i.05, 1
+  br i1 false, label %for.body, label %for.body.6
+
+for.body.6:                                       ; preds = %for.body.6, %for.body
+  %i.14 = phi i32 [ undef, %for.body.6 ], [ 0, %for.body ]
+  br i1 undef, label %for.body.6, label %for.body.16
+
+for.body.16:                                      ; preds = %for.body.16, %for.body.6
+  br i1 undef, label %for.body.16, label %for.body.29
+
+for.body.29:                                      ; preds = %for.body.29, %for.body.16
+  br i1 undef, label %for.body.29, label %for.end.38
+
+for.end.38:                                       ; preds = %for.body.29
+  unreachable
+}

polly/test/Isl/CodeGen/phi_in_exit_early_lnt_failure_2.ll

@@ -0,0 +1,48 @@
+; RUN: opt %loadPolly -polly-detect-unprofitable -polly-codegen -polly-no-early-exit -S < %s | FileCheck %s
+;
+; This caused an lnt crash at some point, just verify it will run through and
+; produce the PHI node in the exit we are looking for.
+;
+; CHECK:       %eps1.addr.0.s2a = alloca double
+; CHECK-NOT:   %eps1.addr.0.ph.s2a = alloca double
+;
+; CHECK-LABEL: polly.merge_new_and_old:
+; CHECK:          %eps1.addr.0.ph.merge = phi double [ %eps1.addr.0.ph.final_reload, %polly.stmt.if.end.47.region_exiting.exit ], [ %eps1.addr.0.ph, %if.end.47.region_exiting ]
+;
+; CHECK-LABEL: polly.start:
+; CHECK-NEXT:    store double %eps1, double* %eps1.s2a
+;
+; CHECK-LABEL: polly.stmt.if.end.47.region_exiting.exit:
+; CEHCK-NEXT:     %eps1.addr.0.ph.final_reload = load double, double* %eps1.addr.0.s2a
+;
+define void @dbisect(double* %c, double* %b, double %eps1, double* %eps2) {
+entry:
+  br label %entry.split
+
+entry.split:                                      ; preds = %entry
+  store double 0.000000e+00, double* %b, align 8
+  br i1 false, label %for.inc, label %for.end
+
+if.end:                                           ; preds = %if.then, %for.body
+  %arrayidx33 = getelementptr inbounds double, double* %c, i64 0
+  %0 = load double, double* %arrayidx33, align 8
+  br label %for.inc
+
+for.inc:                                          ; preds = %if.then.36, %if.end
+  br i1 false, label %if.end, label %for.cond.for.end_crit_edge
+
+for.cond.for.end_crit_edge:                       ; preds = %for.inc
+  br label %for.end
+
+for.end:                                          ; preds = %for.cond.for.end_crit_edge, %entry.split
+  %cmp45 = fcmp ugt double %eps1, 0.000000e+00
+  br i1 %cmp45, label %if.end.47, label %if.then.46
+
+if.then.46:                                       ; preds = %for.end
+  %1 = load double, double* %eps2, align 8
+  br label %if.end.47
+
+if.end.47:                                        ; preds = %if.then.46, %for.end
+  %eps1.addr.0 = phi double [ %1, %if.then.46 ], [ %eps1, %for.end ]
+  ret void
+}