Skip to content

Commit

Permalink
[Delinerization] Require by offset to be zero.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Meinersbur committed Sep 8, 2021
1 parent c4e8a21 commit 088577a
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
4 changes: 2 additions & 2 deletions llvm/lib/Analysis/Delinearization.cpp
Expand Up @@ -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;
Expand Down
39 changes: 39 additions & 0 deletions 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
}

0 comments on commit 088577a

Please sign in to comment.