[llvm] 088577a - [Delinerization] Require by offset to be zero.

[Michael Kruse via llvm-commits]

Author: Michael Kruse
Date: 2021-09-08T16:02:37-05:00
New Revision: 088577a38e603d3537816de1f2a249d598de8dd9

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

LOG: [Delinerization] Require by offset to be zero.

Users of delinearization assume that the the offset into the array element is zero. In most cases it will indeed be zero, but if it is not, the delinearization has to fail since it violates that assumption without the API even allowing to signal to the caller that the by offset is non-zero.

This bug caused Polly to miscompile blender (526.blender_r from SPEC CPU 2017) in -polly-process-unprofitable mode. The SCEV expression incorrectly delinearized has been reduced in the test case byte_offset.ll. The dropped offset into the array element of size 4 (a float) is ((sext i32 %mul7.i4534 to i64) + {(sext i32 %i1 to i64),+,((sext i32 (1 + ((1 + %shl.i.i) * (1 + %shl.i.i)) + %shl.i.i) to i64) * (sext i32 %i1 to i64))}<%for.body703>). This significant component was just dropped, and the wrong pointer was computed when regenerating code from the remaining delinearized subscripts. This occurred during blender's subsurface scattering implementation. As a result, blender's rendering diverged from the reference image.

Patch D108885 would also fix the API.

Reviewed By: bmahjour

Differential Revision: https://reviews.llvm.org/D109133

Added: 
    llvm/test/Analysis/Delinearization/byte_offset.ll

Modified: 
    llvm/lib/Analysis/Delinearization.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/Delinearization.cpp b/llvm/lib/Analysis/Delinearization.cpp
index 8219d59b8f40e..de490ba7ec6e2 100644
--- a/llvm/lib/Analysis/Delinearization.cpp
+++ b/llvm/lib/Analysis/Delinearization.cpp
@@ -373,8 +373,8 @@ void llvm::computeAccessFunctions(ScalarEvolution &SE, const SCEV *Expr,
     // the array.
     if (i == Last) {
 
-      // Bail out if the remainder is too complex.
-      if (isa<SCEVAddRecExpr>(R)) {
+      // Bail out if the byte offset is non-zero.
+      if (!R->isZero()) {
         Subscripts.clear();
         Sizes.clear();
         return;

diff  --git a/llvm/test/Analysis/Delinearization/byte_offset.ll b/llvm/test/Analysis/Delinearization/byte_offset.ll
new file mode 100644
index 0000000000000..9010a314ab894
--- /dev/null
+++ b/llvm/test/Analysis/Delinearization/byte_offset.ll
@@ -0,0 +1,39 @@
+; RUN: opt < %s -analyze -enable-new-pm=0 -delinearize | FileCheck %s
+; RUN: opt < %s -passes='print<delinearization>' -disable-output 2>&1 | FileCheck %s
+
+; CHECK: AccessFunction: ({0,+,%i2}<%outer.loop> + %unknown)
+; CHECK-NEXT: failed to delinearize
+
+; void foo(char A[], long i2, bool c) {
+;   for (long i = 0; ; ++i) {
+;     char *tmp = &A[i * i2];
+;     if (c)
+;       while (1)
+;          *((float*)&tmp[arg << arg]) = 0;
+;   }
+; }
+
+define void @foo(i8* %A, i64 %i2, i64 %arg, i1 %c) {
+entry:
+  br label %outer.loop
+
+outer.loop:
+  %outer.iv = phi i64 [ 0, %entry ], [ %outer.iv.next, %outer.latch ]
+  %i414 = mul nsw i64 %outer.iv, %i2
+  %tmp = getelementptr inbounds i8, i8* %A, i64 %i414
+  br i1 %c, label %inner.preheader, label %outer.latch
+
+inner.preheader:
+  %unknown = shl i64 %arg, %arg
+  %arrayidx = getelementptr inbounds i8, i8* %tmp, i64 %unknown
+  %ptr = bitcast i8* %arrayidx to float*
+  br label %inner.loop
+
+inner.loop:
+  store float 0.000000e+00, float* %ptr, align 4
+  br label %inner.loop
+
+outer.latch:
+  %outer.iv.next = add nuw nsw i64 %outer.iv, 1
+  br label %outer.loop
+}