Add an invalid domain to memory accesses

  Memory accesses can have non-precisely modeled access functions that
  would cause us to build incorrect execution context for hoisted loads.
  This is the same issue that occurred during the domain construction for
  statements and it is dealt with the same way.

llvm-svn: 267289

diff --git a/polly/include/polly/ScopInfo.h b/polly/include/polly/ScopInfo.h
index cf8198b..e90a150 100644 (file)
--- a/polly/include/polly/ScopInfo.h
+++ b/polly/include/polly/ScopInfo.h
@@ -493,6 +493,13 @@ private:
   /// @brief Parent ScopStmt of this access.
   ScopStmt *Statement;
 
+  /// @brief The domain under which this access is not modeled precisely.
+  ///
+  /// The invalid domain for an access describes all parameter combinations
+  /// under which the statement looks to be executed but is in fact not because
+  /// some assumption/restriction makes the access invalid.
+  isl_set *InvalidDomain;
+
   // Properties describing the accessed array.
   // TODO: It might be possible to move them to ScopArrayInfo.
   // @{
@@ -801,8 +808,20 @@ public:
   const SCEV *getSubscript(unsigned Dim) const { return Subscripts[Dim]; }
 
   /// @brief Compute the isl representation for the SCEV @p E wrt. this access.
+  ///
+  /// Note that this function will also adjust the invalid context accordingly.
   __isl_give isl_pw_aff *getPwAff(const SCEV *E);
 
+  /// @brief Get the invalid domain for this access.
+  __isl_give isl_set *getInvalidDomain() const {
+    return isl_set_copy(InvalidDomain);
+  }
+
+  /// @brief Get the invalid context for this access.
+  __isl_give isl_set *getInvalidContext() const {
+    return isl_set_params(getInvalidDomain());
+  }
+
   /// Get the stride of this memory access in the specified Schedule. Schedule
   /// is a map from the statement to a schedule where the innermost dimension is
   /// the dimension of the innermost loop containing the statement.

diff --git a/polly/lib/Analysis/ScopInfo.cpp b/polly/lib/Analysis/ScopInfo.cpp
index e3dba4b..d625d41 100644 (file)
--- a/polly/lib/Analysis/ScopInfo.cpp
+++ b/polly/lib/Analysis/ScopInfo.cpp
@@ -548,6 +548,7 @@ getIndexExpressionsFromGEP(GetElementPtrInst *GEP, ScalarEvolution &SE) {
 
 MemoryAccess::~MemoryAccess() {
   isl_id_free(Id);
+  isl_set_free(InvalidDomain);
   isl_map_free(AccessRelation);
   isl_map_free(NewAccessRelation);
 }
@@ -818,6 +819,10 @@ static bool isDivisible(const SCEV *Expr, unsigned Size, ScalarEvolution &SE) {
 void MemoryAccess::buildAccessRelation(const ScopArrayInfo *SAI) {
   assert(!AccessRelation && "AccessReltation already built");
 
+  // Initialize the invalid domain which describes all iterations for which the
+  // access relation is not modeled correctly.
+  InvalidDomain = getStatement()->getInvalidDomain();
+
   isl_ctx *Ctx = isl_id_get_ctx(Id);
   isl_id *BaseAddrId = SAI->getBasePtrId();
 
@@ -866,9 +871,9 @@ MemoryAccess::MemoryAccess(ScopStmt *Stmt, Instruction *AccessInst,
                            ArrayRef<const SCEV *> Sizes, Value *AccessValue,
                            ScopArrayInfo::MemoryKind Kind, StringRef BaseName)
     : Kind(Kind), AccType(AccType), RedType(RT_NONE), Statement(Stmt),
-      BaseAddr(BaseAddress), BaseName(BaseName), ElementType(ElementType),
-      Sizes(Sizes.begin(), Sizes.end()), AccessInstruction(AccessInst),
-      AccessValue(AccessValue), IsAffine(Affine),
+      InvalidDomain(nullptr), BaseAddr(BaseAddress), BaseName(BaseName),
+      ElementType(ElementType), Sizes(Sizes.begin(), Sizes.end()),
+      AccessInstruction(AccessInst), AccessValue(AccessValue), IsAffine(Affine),
       Subscripts(Subscripts.begin(), Subscripts.end()), AccessRelation(nullptr),
       NewAccessRelation(nullptr) {
   static const std::string TypeStrings[] = {"", "_Read", "_Write", "_MayWrite"};
@@ -881,6 +886,8 @@ MemoryAccess::MemoryAccess(ScopStmt *Stmt, Instruction *AccessInst,
 
 void MemoryAccess::realignParams() {
   isl_space *ParamSpace = Statement->getParent()->getParamSpace();
+  InvalidDomain =
+      isl_set_align_params(InvalidDomain, isl_space_copy(ParamSpace));
   AccessRelation = isl_map_align_params(AccessRelation, ParamSpace);
 }
 
@@ -922,7 +929,9 @@ void MemoryAccess::dump() const { print(errs()); }
 
 __isl_give isl_pw_aff *MemoryAccess::getPwAff(const SCEV *E) {
   auto *Stmt = getStatement();
-  return Stmt->getParent()->getPwAffOnly(E, Stmt->getEntryBlock());
+  PWACtx PWAC = Stmt->getParent()->getPwAff(E, Stmt->getEntryBlock());
+  InvalidDomain = isl_set_union(InvalidDomain, PWAC.second);
+  return PWAC.first;
 }
 
 // Create a map in the size of the provided set domain, that maps from the
@@ -3192,7 +3201,8 @@ const InvariantEquivClassTy *Scop::lookupInvariantEquivClass(Value *Val) const {
 }
 
 /// @brief Check if @p MA can always be hoisted without execution context.
-static bool canAlwaysBeHoisted(MemoryAccess *MA, bool StmtInvalidCtxIsEmpty) {
+static bool canAlwaysBeHoisted(MemoryAccess *MA, bool StmtInvalidCtxIsEmpty,
+                               bool MAInvalidCtxIsEmpty) {
   LoadInst *LInst = cast<LoadInst>(MA->getAccessInstruction());
   const DataLayout &DL = LInst->getParent()->getModule()->getDataLayout();
   // TODO: We can provide more information for better but more expensive
@@ -3203,7 +3213,7 @@ static bool canAlwaysBeHoisted(MemoryAccess *MA, bool StmtInvalidCtxIsEmpty) {
 
   // If a dereferencable load is in a statement that is modeled precisely we can
   // hoist it.
-  if (StmtInvalidCtxIsEmpty)
+  if (StmtInvalidCtxIsEmpty && MAInvalidCtxIsEmpty)
     return true;
 
   // Even if the statement is not modeled precisely we can hoist the load if it
@@ -3267,12 +3277,17 @@ void Scop::addInvariantLoads(ScopStmt &Stmt, MemoryAccessList &InvMAs) {
     Type *Ty = LInst->getType();
     const SCEV *PointerSCEV = SE->getSCEV(LInst->getPointerOperand());
 
+    auto *MAInvalidCtx = MA->getInvalidContext();
+    bool MAInvalidCtxIsEmpty = isl_set_is_empty(MAInvalidCtx);
+
     isl_set *MACtx;
     // Check if we know that this pointer can be speculatively accessed.
-    if (canAlwaysBeHoisted(MA, StmtInvalidCtxIsEmpty)) {
+    if (canAlwaysBeHoisted(MA, StmtInvalidCtxIsEmpty, MAInvalidCtxIsEmpty)) {
       MACtx = isl_set_universe(isl_set_get_space(DomainCtx));
+      isl_set_free(MAInvalidCtx);
     } else {
       MACtx = isl_set_copy(DomainCtx);
+      MACtx = isl_set_subtract(MACtx, MAInvalidCtx);
       MACtx = isl_set_gist_params(MACtx, getContext());
     }
 

diff --git a/polly/test/ScopInfo/non-precise-inv-load-1.ll b/polly/test/ScopInfo/non-precise-inv-load-1.ll
new file mode 100644 (file)
index 0000000..8ecd30b
--- /dev/null
+++ b/polly/test/ScopInfo/non-precise-inv-load-1.ll
@@ -0,0 +1,53 @@
+; RUN: opt %loadPolly -polly-scops -analyze < %s | FileCheck %s
+;
+; Verify we do hoist the invariant access to I with a execution context
+; as the address computation might wrap in the original but not in our
+; optimized version. For an input of c = 127 the original accessed address
+; would be &I[-1] = &GI[128 -1] = &GI[127] but in our optimized version
+; (due to the usage of i64 types) we would access
+; &I[127 + 1] = &I[128] = &GI[256] which would here also be out-of-bounds.
+;
+; CHECK:        Invariant Accesses: {
+; CHECK-NEXT:     ReadAccess := [Reduction Type: NONE] [Scalar: 0]
+; CHECK-NEXT:       [c] -> { Stmt_for_body[i0] -> MemRef_GI[129 + c] };
+; CHECK-NEXT:     Execution Context: [c] -> {  : c <= 126 }
+; CHECK-NEXT:   }
+;
+;    int GI[256];
+;    void f(int *A, unsigned char c) {
+;      int *I = &GI[128];
+;      for (int i = 0; i < 10; i++)
+;        A[i] += I[(signed char)(c + (unsigned char)1)];
+;    }
+;
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+@GI = common global [256 x i32] zeroinitializer, align 16
+
+define void @f(i32* %A, i8 zeroext %c) {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.inc, %entry
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.inc ], [ 0, %entry ]
+  %exitcond = icmp ne i64 %indvars.iv, 10
+  br i1 %exitcond, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %add = add i8 %c, 1
+  %idxprom = sext i8 %add to i64
+  %arrayidx = getelementptr inbounds i32, i32* getelementptr inbounds ([256 x i32], [256 x i32]* @GI, i64 0, i64 128), i64 %idxprom
+  %tmp = load i32, i32* %arrayidx, align 4
+  %arrayidx3 = getelementptr inbounds i32, i32* %A, i64 %indvars.iv
+  %tmp1 = load i32, i32* %arrayidx3, align 4
+  %add4 = add nsw i32 %tmp1, %tmp
+  store i32 %add4, i32* %arrayidx3, align 4
+  br label %for.inc
+
+for.inc:                                          ; preds = %for.body
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  br label %for.cond
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}