[Polly] Track defined behavior for PHI predecessor computation.

ZoneAlgorithms's computePHI relies on being provided with consistent a
schedule to compute the statement prodecessors of a statement containing
PHINodes. Otherwise unexpected results such as PHI nodes with multiple
predecessors can occur which would result in problems in the
algorithms expecting consistent data.

In the added test case, statement instances are scrubbed from the
SCoP their execution would result in undefined behavior (Due to a nsw
overflow). As already being undefined behavior in LLVM-IR, neither
AssumedContext nor InvalidContext are updated, giving computePHI no
means to avoid these cases.

Intoduce a new SCoP property, the DefinedBehaviorContext, that among
the runtime-checked conditions, also tracks the assumptions not needing
a runtime check, in particular those affecting the assumed control flow.
This replaces the manual combination of the 3 other contexts that was
already done in computePHI and setNewAccessRelation. Currently, the only
additional assumption is that loop induction variables will nsw flag for
not wrap, but potentially more can be added. Use in
hasFeasibleRuntimeContext, isl::ast_build and gisting are other
potential uses.

To limit computational complexity, the DefinedBehaviorContext is not
availabe if it grows too large (atm hardcoded to 8 disjuncts).

Possible other fixes include bailing out in computePHI when
inconsistencies are detected, choose an arbitrary value for inconsistent
cases (since it is undefined behavior anyways), or make the code
receiving the result from ComputePHI handle inconsistent data. All of
them reduce the quality of implementation having to bail out more often
and disabling the ability to assert on actually wrong results.

This fixes llvm.org/PR48783.

Author: Meinersbur

polly/include/polly/ScopInfo.h

@@ -1805,6 +1805,22 @@ class Scop {
   /// need to be "false". Otherwise they behave the same.
   isl::set InvalidContext;
 
+  /// The context under which the SCoP must have defined behavior. Optimizer and
+  /// code generator can assume that the SCoP will only be executed with
+  /// parameter values within this context. This might be either because we can
+  /// prove that other values are impossible or explicitly have undefined
+  /// behavior, such as due to no-wrap flags. If this becomes too complex, can
+  /// also be nullptr.
+  ///
+  /// In contrast to Scop::AssumedContext and Scop::InvalidContext, these do not
+  /// need to be checked at runtime.
+  ///
+  /// Scop::Context on the other side is an overapproximation and does not
+  /// include all requirements, but is always defined. However, there is still
+  /// no guarantee that there is no undefined behavior in
+  /// DefinedBehaviorContext.
+  isl::set DefinedBehaviorContext;
+
   /// The schedule of the SCoP
   ///
   /// The schedule of the SCoP describes the execution order of the statements
@@ -2200,6 +2216,19 @@ class Scop {
   /// @return The constraint on parameter of this Scop.
   isl::set getContext() const;
 
+  /// Return the context where execution behavior is defined. Might return
+  /// nullptr.
+  isl::set getDefinedBehaviorContext() const { return DefinedBehaviorContext; }
+
+  /// Return the define behavior context, or if not available, its approximation
+  /// from all other contexts.
+  isl::set getBestKnownDefinedBehaviorContext() const {
+    if (DefinedBehaviorContext)
+      return DefinedBehaviorContext;
+
+    return Context.intersect_params(AssumedContext).subtract(InvalidContext);
+  }
+
   /// Return space of isl context parameters.
   ///
   /// Returns the set of context parameters that are currently constrained. In
@@ -2254,6 +2283,10 @@ class Scop {
   bool trackAssumption(AssumptionKind Kind, isl::set Set, DebugLoc Loc,
                        AssumptionSign Sign, BasicBlock *BB);
 
+  /// Add the conditions from @p Set (or subtract them if @p Sign is
+  /// AS_RESTRICTION) to the defined behaviour context.
+  void intersectDefinedBehavior(isl::set Set, AssumptionSign Sign);
+
   /// Add assumptions to assumed context.
   ///
   /// The assumptions added will be assumed to hold during the execution of the
@@ -2272,8 +2305,9 @@ class Scop {
   ///             (needed/assumptions) or negative (invalid/restrictions).
   /// @param BB   The block in which this assumption was taken. Used to
   ///             calculate hotness when emitting remark.
+  /// @param RTC  Does the assumption require a runtime check?
   void addAssumption(AssumptionKind Kind, isl::set Set, DebugLoc Loc,
-                     AssumptionSign Sign, BasicBlock *BB);
+                     AssumptionSign Sign, BasicBlock *BB, bool RTC = true);
 
   /// Mark the scop as invalid.
   ///

polly/include/polly/Support/ScopHelper.h

@@ -68,6 +68,9 @@ struct Assumption {
 
   /// An optional block whose domain can simplify the assumption.
   llvm::BasicBlock *BB;
+
+  // Whether the assumption must be checked at runtime.
+  bool RequiresRTC;
 };
 
 using RecordedAssumptionsTy = llvm::SmallVector<Assumption, 8>;
@@ -88,9 +91,11 @@ using RecordedAssumptionsTy = llvm::SmallVector<Assumption, 8>;
 ///             set, the domain of that block will be used to simplify the
 ///             actual assumption in @p Set once it is added. This is useful
 ///             if the assumption was created prior to the domain.
+/// @param RTC  Does the assumption require a runtime check?
 void recordAssumption(RecordedAssumptionsTy *RecordedAssumptions,
                       AssumptionKind Kind, isl::set Set, llvm::DebugLoc Loc,
-                      AssumptionSign Sign, llvm::BasicBlock *BB = nullptr);
+                      AssumptionSign Sign, llvm::BasicBlock *BB = nullptr,
+                      bool RTC = true);
 
 /// Type to remap values.
 using ValueMapT = llvm::DenseMap<llvm::AssertingVH<llvm::Value>,

polly/lib/Analysis/ScopBuilder.cpp

@@ -794,15 +794,15 @@ bool ScopBuilder::addLoopBoundsToHeaderDomain(
   auto Parts = partitionSetParts(HeaderBBDom, LoopDepth);
   HeaderBBDom = Parts.second;
 
-  // Check if there is a <nsw> tagged AddRec for this loop and if so do not add
-  // the bounded assumptions to the context as they are already implied by the
-  // <nsw> tag.
-  if (scop->hasNSWAddRecForLoop(L))
-    return true;
+  // Check if there is a <nsw> tagged AddRec for this loop and if so do not
+  // require a runtime check. The assumption is already implied by the <nsw>
+  // tag.
+  bool RequiresRTC = !scop->hasNSWAddRecForLoop(L);
 
   isl::set UnboundedCtx = Parts.first.params();
   recordAssumption(&RecordedAssumptions, INFINITELOOP, UnboundedCtx,
-                   HeaderBB->getTerminator()->getDebugLoc(), AS_RESTRICTION);
+                   HeaderBB->getTerminator()->getDebugLoc(), AS_RESTRICTION,
+                   nullptr, RequiresRTC);
   return true;
 }
 
@@ -1495,7 +1495,7 @@ void ScopBuilder::addRecordedAssumptions() {
 
     if (!AS.BB) {
       scop->addAssumption(AS.Kind, AS.Set, AS.Loc, AS.Sign,
-                          nullptr /* BasicBlock */);
+                          nullptr /* BasicBlock */, AS.RequiresRTC);
       continue;
     }
 
@@ -1519,7 +1519,8 @@ void ScopBuilder::addRecordedAssumptions() {
     else /* (AS.Sign == AS_ASSUMPTION) */
       S = isl_set_params(isl_set_subtract(Dom, S));
 
-    scop->addAssumption(AS.Kind, isl::manage(S), AS.Loc, AS_RESTRICTION, AS.BB);
+    scop->addAssumption(AS.Kind, isl::manage(S), AS.Loc, AS_RESTRICTION, AS.BB,
+                        AS.RequiresRTC);
   }
 }
 

polly/lib/Analysis/ScopInfo.cpp

@@ -118,6 +118,10 @@ int const polly::MaxDisjunctsInDomain = 20;
 // number of disjunct when adding non-convex sets to the context.
 static int const MaxDisjunctsInContext = 4;
 
+// Be a bit more generous for the defined behavior context which is used less
+// often.
+static int const MaxDisjunktsInDefinedBehaviourContext = 8;
+
 static cl::opt<bool> PollyRemarksMinimal(
     "polly-remarks-minimal",
     cl::desc("Do not emit remarks about assumptions that are known"),
@@ -1077,12 +1081,11 @@ void MemoryAccess::setNewAccessRelation(isl::map NewAccess) {
   if (isRead()) {
     // Check whether there is an access for every statement instance.
     isl::set StmtDomain = getStatement()->getDomain();
-    StmtDomain =
-        StmtDomain.intersect_params(getStatement()->getParent()->getContext());
-    StmtDomain = subtractParams(
-        StmtDomain, getStatement()->getParent()->getInvalidContext());
+    isl::set DefinedContext =
+        getStatement()->getParent()->getBestKnownDefinedBehaviorContext();
+    StmtDomain = StmtDomain.intersect_params(DefinedContext);
     isl::set NewDomain = NewAccess.domain();
-    assert(StmtDomain.is_subset(NewDomain) &&
+    assert(!StmtDomain.is_subset(NewDomain).is_false() &&
            "Partial READ accesses not supported");
   }
 
@@ -1561,6 +1564,7 @@ void Scop::buildContext() {
   Context = isl::set::universe(Space);
   InvalidContext = isl::set::empty(Space);
   AssumedContext = isl::set::universe(Space);
+  DefinedBehaviorContext = isl::set::universe(Space);
 }
 
 void Scop::addParameterBounds() {
@@ -1569,6 +1573,7 @@ void Scop::addParameterBounds() {
     ConstantRange SRange = SE->getSignedRange(Parameter);
     Context = addRangeBoundsToSet(Context, SRange, PDim++, isl::dim::param);
   }
+  intersectDefinedBehavior(Context, AS_ASSUMPTION);
 }
 
 static std::vector<isl::id> getFortranArrayIds(Scop::array_range Arrays) {
@@ -1682,6 +1687,8 @@ void Scop::simplifyContexts() {
   //   only executed for the case m >= 0, it is sufficient to assume p >= 0.
   AssumedContext = simplifyAssumptionContext(AssumedContext, *this);
   InvalidContext = InvalidContext.align_params(getParamSpace());
+  simplify(DefinedBehaviorContext);
+  DefinedBehaviorContext = DefinedBehaviorContext.align_params(getParamSpace());
 }
 
 isl::set Scop::getDomainConditions(const ScopStmt *Stmt) const {
@@ -2109,9 +2116,14 @@ bool Scop::trackAssumption(AssumptionKind Kind, isl::set Set, DebugLoc Loc,
 }
 
 void Scop::addAssumption(AssumptionKind Kind, isl::set Set, DebugLoc Loc,
-                         AssumptionSign Sign, BasicBlock *BB) {
+                         AssumptionSign Sign, BasicBlock *BB,
+                         bool RequiresRTC) {
   // Simplify the assumptions/restrictions first.
   Set = Set.gist_params(getContext());
+  intersectDefinedBehavior(Set, Sign);
+
+  if (!RequiresRTC)
+    return;
 
   if (!trackAssumption(Kind, Set, Loc, Sign, BB))
     return;
@@ -2122,6 +2134,26 @@ void Scop::addAssumption(AssumptionKind Kind, isl::set Set, DebugLoc Loc,
     InvalidContext = InvalidContext.unite(Set).coalesce();
 }
 
+void Scop::intersectDefinedBehavior(isl::set Set, AssumptionSign Sign) {
+  if (!DefinedBehaviorContext)
+    return;
+
+  if (Sign == AS_ASSUMPTION)
+    DefinedBehaviorContext = DefinedBehaviorContext.intersect(Set);
+  else
+    DefinedBehaviorContext = DefinedBehaviorContext.subtract(Set);
+
+  // Limit the complexity of the context. If complexity is exceeded, simplify
+  // the set and check again.
+  if (DefinedBehaviorContext.n_basic_set() >
+      MaxDisjunktsInDefinedBehaviourContext) {
+    simplify(DefinedBehaviorContext);
+    if (DefinedBehaviorContext.n_basic_set() >
+        MaxDisjunktsInDefinedBehaviourContext)
+      DefinedBehaviorContext = nullptr;
+  }
+}
+
 void Scop::invalidate(AssumptionKind Kind, DebugLoc Loc, BasicBlock *BB) {
   LLVM_DEBUG(dbgs() << "Invalidate SCoP because of reason " << Kind << "\n");
   addAssumption(Kind, isl::set::empty(getParamSpace()), Loc, AS_ASSUMPTION, BB);
@@ -2139,6 +2171,12 @@ void Scop::printContext(raw_ostream &OS) const {
   OS.indent(4) << "Invalid Context:\n";
   OS.indent(4) << InvalidContext << "\n";
 
+  OS.indent(4) << "Defined Behavior Context:\n";
+  if (DefinedBehaviorContext)
+    OS.indent(4) << DefinedBehaviorContext << "\n";
+  else
+    OS.indent(4) << "<unavailable>\n";
+
   unsigned Dim = 0;
   for (const SCEV *Parameter : Parameters)
     OS.indent(4) << "p" << Dim++ << ": " << *Parameter << "\n";

polly/lib/Support/ScopHelper.cpp

@@ -224,11 +224,11 @@ void polly::splitEntryBlockForAlloca(BasicBlock *EntryBlock, Pass *P) {
 void polly::recordAssumption(polly::RecordedAssumptionsTy *RecordedAssumptions,
                              polly::AssumptionKind Kind, isl::set Set,
                              DebugLoc Loc, polly::AssumptionSign Sign,
-                             BasicBlock *BB) {
+                             BasicBlock *BB, bool RTC) {
   assert((Set.is_params() || BB) &&
          "Assumptions without a basic block must be parameter sets");
   if (RecordedAssumptions)
-    RecordedAssumptions->push_back({Kind, Sign, Set, Loc, BB});
+    RecordedAssumptions->push_back({Kind, Sign, Set, Loc, BB, RTC});
 }
 
 /// The SCEVExpander will __not__ generate any code for an existing SDiv/SRem

polly/lib/Transform/DeLICM.cpp

@@ -868,6 +868,11 @@ class DeLICMImpl : public ZoneAlgorithm {
 
     // { DomainRead[] -> DomainWrite[] }
     auto PerPHIWrites = computePerPHI(SAI);
+    if (!PerPHIWrites) {
+      LLVM_DEBUG(
+          dbgs() << "    Reject because cannot determine incoming values\n");
+      return false;
+    }
 
     // { DomainWrite[] -> Element[] }
     auto WritesTarget = PerPHIWrites.apply_domain(PHITarget).reverse();

polly/lib/Transform/ForwardOpTree.cpp

@@ -306,6 +306,9 @@ class ForwardOpTreeImpl : ZoneAlgorithm {
     // { Domain[] -> Element[] }
     isl::map Result;
 
+    // Make irrelevant elements not interfere.
+    Domain = Domain.intersect_params(S->getContext());
+
     // MemoryAccesses can read only elements from a single array
     // (i.e. not: { Dom[0] -> A[0]; Dom[1] -> B[1] }).
     // Look through all spaces until we find one that contains at least the

polly/lib/Transform/ZoneAlgo.cpp

@@ -541,6 +541,13 @@ isl::union_map ZoneAlgorithm::computePerPHI(const ScopArrayInfo *SAI) {
   if (It != PerPHIMaps.end())
     return It->second;
 
+  // Cannot reliably compute immediate predecessor for undefined executions, so
+  // bail out if we do not know. This in particular applies to undefined control
+  // flow.
+  isl::set DefinedContext = S->getDefinedBehaviorContext();
+  if (!DefinedContext)
+    return nullptr;
+
   assert(SAI->isPHIKind());
 
   // { DomainPHIWrite[] -> Scatter[] }
@@ -565,8 +572,7 @@ isl::union_map ZoneAlgorithm::computePerPHI(const ScopArrayInfo *SAI) {
   isl::map PHIWriteTimes = BeforeRead.intersect_range(WriteTimes);
 
   // Remove instances outside the context.
-  PHIWriteTimes = PHIWriteTimes.intersect_params(S->getAssumedContext());
-  PHIWriteTimes = subtractParams(PHIWriteTimes, S->getInvalidContext());
+  PHIWriteTimes = PHIWriteTimes.intersect_params(DefinedContext);
 
   isl::map LastPerPHIWrites = PHIWriteTimes.lexmax();
 
@@ -1025,6 +1031,13 @@ void ZoneAlgorithm::computeNormalizedPHIs() {
       auto *PHI = cast<PHINode>(MA->getAccessInstruction());
       const ScopArrayInfo *SAI = MA->getOriginalScopArrayInfo();
 
+      // Determine which instance of the PHI statement corresponds to which
+      // incoming value. Skip if we cannot determine PHI predecessors.
+      // { PHIDomain[] -> IncomingDomain[] }
+      isl::union_map PerPHI = computePerPHI(SAI);
+      if (!PerPHI)
+        continue;
+
       // { PHIDomain[] -> PHIValInst[] }
       isl::map PHIValInst = makeValInst(PHI, &Stmt, Stmt.getSurroundingLoop());
 
@@ -1048,11 +1061,6 @@ void ZoneAlgorithm::computeNormalizedPHIs() {
         IncomingValInsts = IncomingValInsts.add_map(IncomingValInst);
       }
 
-      // Determine which instance of the PHI statement corresponds to which
-      // incoming value.
-      // { PHIDomain[] -> IncomingDomain[] }
-      isl::union_map PerPHI = computePerPHI(SAI);
-
       // { PHIValInst[] -> IncomingValInst[] }
       isl::union_map PHIMap =
           PerPHI.apply_domain(PHIValInst).apply_range(IncomingValInsts);

polly/test/DeLICM/pr41656.ll

@@ -1,4 +1,4 @@
-; RUN: opt %loadPolly -polly-delicm -analyze < %s | FileCheck %s
+; RUN: opt %loadPolly -polly-scops -polly-delicm -analyze < %s | FileCheck %s
 ;
 ; llvm.org/PR41656
 ; 
@@ -81,8 +81,14 @@ attributes #2 = { nounwind }
 !6 = !{!"double", !3, i64 0}
 
 
+; CHECK:      Invalid Context:
+; CHECK-NEXT: [call24] -> {  : call24 <= 2 }
+; CHECK:      Defined Behavior Context:
+; CHECK-NEXT: [call24] -> {  : 3 <= call24 <= 2147483647 }
+
 ; Only write to scalar if call24 >= 3 (i.e. not in invalid context)
-; Since it should be never executed otherwise, the condition is not strictly necessary. 
+; Since it should be never executed otherwise, the condition is not strictly necessary.
+; CHECK-LABEL: DeLICM result:
 ; CHECK:          Stmt_for_body_us_preheader_i
 ; CHECK-NEXT:            MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 1]
 ; CHECK-NEXT:                 [call24] -> { Stmt_for_body_us_preheader_i[] -> MemRef_t_1__phi[] };