[llvm-branch-commits] [polly] bc633fe - [Polly] Consider InvalidContext to determine partial READ.

Michael Kruse via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Dec 10 20:33:07 PST 2020


Author: Michael Kruse
Date: 2020-12-10T22:25:19-06:00
New Revision: bc633fe46bbec1cf70d4a3f740da43888979d8c6

URL: https://github.com/llvm/llvm-project/commit/bc633fe46bbec1cf70d4a3f740da43888979d8c6
DIFF: https://github.com/llvm/llvm-project/commit/bc633fe46bbec1cf70d4a3f740da43888979d8c6.diff

LOG: [Polly] Consider InvalidContext to determine partial READ.

MemoryAccess::setNewAccessRelation() in assert-builds checks whether the
access relation for a READ has a memory location for every instance of
the domain. Otherwise, we would not have value to load from. That check
already considered that instances outside the Scop's context do not
matter since they are never executed (or would be undefined behavior).
In this patch also take instances of the InvalidContext into account,
as these can also be assumed to never occur. InvalidContext was
introduced to avoid the computational complexity of subtracting
restrictions from the AssumedContext. However, this additional check in
setNewAccessRelation is only done in assert-builds.

The assertion case with an InvalidContext may occur with DeLICM on a
conditionally infinite loops, as it is the case in the following code:

    for (int i = 0; i < n; i+=b)
      vreg = ...;
    *Dest = vreg;

The loop is infinite when b=0, and [b] -> { : b = 0 }  is part of the
InvalidContext. When DeLICM tries to map the memory for %vreg to *Dest,
there is no store instance that uses the value of vreg when b = 0, hence
no location to map it to. However, the case is irrelevant since Polly's
runtime condition check ensures that this is never case.

Fixes llvm.org/PR48445

Added: 
    polly/test/DeLICM/load-in-cond-inf-loop.ll

Modified: 
    polly/include/polly/Support/ISLTools.h
    polly/lib/Analysis/ScopInfo.cpp
    polly/lib/Support/ISLTools.cpp

Removed: 
    


################################################################################
diff  --git a/polly/include/polly/Support/ISLTools.h b/polly/include/polly/Support/ISLTools.h
index 41a9021896bc..1e028313153b 100644
--- a/polly/include/polly/Support/ISLTools.h
+++ b/polly/include/polly/Support/ISLTools.h
@@ -478,6 +478,9 @@ isl::map intersectRange(isl::map Map, isl::union_set Range);
 /// @param The map with the parameter conditions removed.
 isl::map subtractParams(isl::map Map, isl::set Params);
 
+/// Subtract the parameter space @p Params from @p Set.
+isl::set subtractParams(isl::set Set, isl::set Params);
+
 /// If @p PwAff maps to a constant, return said constant. If @p Max/@p Min, it
 /// can also be a piecewise constant and it would return the minimum/maximum
 /// value. Otherwise, return NaN.

diff  --git a/polly/lib/Analysis/ScopInfo.cpp b/polly/lib/Analysis/ScopInfo.cpp
index ba462351af57..97321c018a12 100644
--- a/polly/lib/Analysis/ScopInfo.cpp
+++ b/polly/lib/Analysis/ScopInfo.cpp
@@ -1079,6 +1079,8 @@ void MemoryAccess::setNewAccessRelation(isl::map NewAccess) {
     isl::set StmtDomain = getStatement()->getDomain();
     StmtDomain =
         StmtDomain.intersect_params(getStatement()->getParent()->getContext());
+    StmtDomain = subtractParams(
+        StmtDomain, getStatement()->getParent()->getInvalidContext());
     isl::set NewDomain = NewAccess.domain();
     assert(StmtDomain.is_subset(NewDomain) &&
            "Partial READ accesses not supported");

diff  --git a/polly/lib/Support/ISLTools.cpp b/polly/lib/Support/ISLTools.cpp
index a7c10314c084..9d8278ac669f 100644
--- a/polly/lib/Support/ISLTools.cpp
+++ b/polly/lib/Support/ISLTools.cpp
@@ -524,6 +524,12 @@ isl::map polly::subtractParams(isl::map Map, isl::set Params) {
   return Map.subtract(ParamsMap);
 }
 
+isl::set polly::subtractParams(isl::set Set, isl::set Params) {
+  isl::space SetSpace = Set.get_space();
+  isl::set ParamsSet = isl::set::universe(SetSpace).intersect_params(Params);
+  return Set.subtract(ParamsSet);
+}
+
 isl::val polly::getConstant(isl::pw_aff PwAff, bool Max, bool Min) {
   assert(!Max || !Min); // Cannot return min and max at the same time.
   isl::val Result;

diff  --git a/polly/test/DeLICM/load-in-cond-inf-loop.ll b/polly/test/DeLICM/load-in-cond-inf-loop.ll
new file mode 100644
index 000000000000..d32d975b3262
--- /dev/null
+++ b/polly/test/DeLICM/load-in-cond-inf-loop.ll
@@ -0,0 +1,71 @@
+; RUN: opt %loadPolly -polly-delicm -analyze < %s | FileCheck %s
+
+; When %b is 0, %for.body13 is an infite loop. In this case the loaded
+; value %1 is not used anywhere.
+; This is a problem when DeLICM tries to map %1 to %arrayidx16 because
+; %1 has no corresponding when %b == 0 and therefore hat no location
+; where it can be mapped to. However, since %b == 0 results in an
+; infinite loop, it should not in the Context, or in this case, in the
+; InvalidContext.
+;
+; Test case reduced from llvm.org/PR48445.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+ at arr_18 = external dso_local local_unnamed_addr global [0 x i16], align 2
+
+define void @func(i64 %b, i8* %c) {
+entry:
+  %conv1 = trunc i64 %b to i32
+  %sext = shl i32 %conv1, 24
+  %conv2 = ashr exact i32 %sext, 24
+  %arrayidx = getelementptr inbounds i8, i8* %c, i64 %b
+  %tobool19.not = icmp eq i64 %b, 0
+  br label %for.cond3.preheader
+
+for.cond3.preheader:
+  %d.039 = phi i16 [ 0, %entry ], [ %inc, %for.cond.cleanup6 ]
+  %idxprom = sext i16 %d.039 to i64
+  br label %for.cond8.preheader
+
+for.cond8.preheader:
+  br label %for.body13
+
+for.cond.cleanup6:
+  %arrayidx16 = getelementptr inbounds [0 x i16], [0 x i16]* @arr_18, i64 0, i64 %idxprom
+  %0 = zext i8 %1 to i16
+  store i16 %0, i16* %arrayidx16, align 2
+  %inc = add i16 %d.039, 1
+  %conv = sext i16 %inc to i32
+  %cmp = icmp sgt i32 %conv2, %conv
+  br i1 %cmp, label %for.cond3.preheader, label %for.cond.cleanup
+
+for.cond.cleanup12:
+  br i1 false, label %for.cond8.preheader, label %for.cond.cleanup6
+
+for.body13:
+  %1 = load i8, i8* %arrayidx, align 1
+  br i1 %tobool19.not, label %for.body13, label %for.cond.cleanup12
+
+for.cond.cleanup:
+  ret void
+}
+
+
+; CHECK: Statistics {
+; CHECK:     Value scalars mapped:  1
+; CHECK: }
+; CHECK:      After accesses {
+; CHECK-NEXT:     Stmt_for_body13
+; CHECK-NEXT:             ReadAccess :=       [Reduction Type: NONE] [Scalar: 0]
+; CHECK-NEXT:                 [b] -> { Stmt_for_body13[i0, i1, i2] -> MemRef_c[b] };
+; CHECK-NEXT:             MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 1]
+; CHECK-NEXT:                 [b] -> { Stmt_for_body13[i0, i1, i2] -> MemRef1[] };
+; CHECK-NEXT:            new: [b] -> { Stmt_for_body13[i0, i1, i2] -> MemRef_arr_18[i0] : i0 < b; Stmt_for_body13[0, i1, i2] -> MemRef_arr_18[0] : b < 0 };
+; CHECK-NEXT:     Stmt_for_cond_cleanup6
+; CHECK-NEXT:             ReadAccess :=       [Reduction Type: NONE] [Scalar: 1]
+; CHECK-NEXT:                 [b] -> { Stmt_for_cond_cleanup6[i0] -> MemRef1[] };
+; CHECK-NEXT:            new: [b] -> { Stmt_for_cond_cleanup6[i0] -> MemRef_arr_18[i0] : i0 < b; Stmt_for_cond_cleanup6[0] -> MemRef_arr_18[0] : b < 0 };
+; CHECK-NEXT:             MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 0]
+; CHECK-NEXT:                 [b] -> { Stmt_for_cond_cleanup6[i0] -> MemRef_arr_18[i0] };
+; CHECK-NEXT: }


        


More information about the llvm-branch-commits mailing list