Fix handling of signWrappedSets in access relations

[maxf]

Summary

Since r294891, in MemoryAccess::computeBoundsOnAccessRelation(), we skip
manually bounding the access relation in case the parameter of the load
instruction is already a wrapped set. Later on we assume that the lower
bound on the set is always smaller or equal to the upper bound on the
set. Bug 32715 manages to construct a sign wrapped set, in which case
the assertion does not necessarily hold. Fix this by handling a sign
wrapped set similar to a normal wrapped set, that is skipping the
computation.

[grosser]

Hi Matthias,

this patch looks very good. I had three minor remarks (see below). As they were very minor, I addressed them directly and immediately committed your patch. These changes are available from r302231.

test/ScopInfo/sign_wrapped_set.ll

7 ↗

(On Diff #97916)

I would also check the memory access functions here.

20 ↗

(On Diff #97916)

Trailing whitespaces.

24 ↗

(On Diff #97916)

Trailing whitespaces.

[sabuasal]

Hi,

This is probably a late comment,

Why are we using "Range.isWrappedSet() | Range.isSignWrappedSet(" instead of the shorthand "Range.isWrappedSet() || Range.isSignWrappedSet("

[grosser]

I do not see a specific reason why we prefer "|" over "||". Just now saw the difference. Maxililian, what is your opinion here?

[maxf]

Hi everyone,

That would have been an oversight on my part, there is no specific reason. Shall I reopen?

[sabuasal]

It is not a big problem really but it just gives the impression that you always want the "Range.isSignWrappedSet" to be executed which is confusing since its an accessor const function anyway.

[sabuasal]

https://reviews.llvm.org/D33337

polly/trunk/lib/Analysis/ScopInfo.cpp

void MemoryAccess::computeBoundsOnAccessRelation(unsigned ElementSize) {
   auto *BasePtrSCEV = SE->getPointerBase(PtrSCEV);
   if (BasePtrSCEV && !isa<SCEVCouldNotCompute>(BasePtrSCEV))
     PtrSCEV = SE->getMinusSCEV(PtrSCEV, BasePtrSCEV);
 
   const ConstantRange &Range = SE->getSignedRange(PtrSCEV);
   if (Range.isFullSet())
     return;
 
-  if (Range.isWrappedSet())
+  if (Range.isWrappedSet() | Range.isSignWrappedSet())
     return;
 
   bool isWrapping = Range.isSignWrappedSet();
 
   unsigned BW = Range.getBitWidth();
   const auto One = APInt(BW, 1);
   const auto LB = isWrapping ? Range.getLower() : Range.getSignedMin();
   const auto UB = isWrapping ? (Range.getUpper() - One) : Range.getSignedMax();

polly/trunk/test/ScopInfo/sign_wrapped_set.ll

+; RUN: opt %loadPolly -polly-scops -analyze \
+; RUN: -polly-allow-nonaffine -polly-process-unprofitable < %s | FileCheck %s
+;
+; CHECK:        Domain :=
+; CHECK-NEXT:       [srcHeight] -> { Stmt_for_cond6_preheader_us[i0] : 0 <= i0 <= -3 + srcHeight };
+; CHECK-NEXT:   Schedule :=
+; CHECK-NEXT:       [srcHeight] -> { Stmt_for_cond6_preheader_us[i0] -> [i0] };
+; CHECK-NEXT:   ReadAccess := [Reduction Type: +] [Scalar: 0]
+; CHECK-NEXT:       [srcHeight] -> { Stmt_for_cond6_preheader_us[i0] -> MemRef_src[o0] };
+; CHECK-NEXT:   MayWriteAccess := [Reduction Type: +] [Scalar: 0]
+; CHECK-NEXT:       [srcHeight] -> { Stmt_for_cond6_preheader_us[i0] -> MemRef_src[o0] };
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+
+define void @test_case(i8* noalias nocapture readonly %src, i32 %srcHeight, i32 %srcStride) local_unnamed_addr {
+entry:
+  %extended = zext i32 %srcStride to i64
+  %sub = add i32 %srcHeight, -1
+  br label %for.cond6.preheader.us
+
+for.cond6.preheader.us:                           ; preds = %for.cond6.preheader.us, %entry
+  %srcPtr.075.us.pn = phi i8* [ %srcPtr.075.us, %for.cond6.preheader.us ], [ %src, %entry ]
+  %y.072.us = phi i32 [ %inc37.us, %for.cond6.preheader.us ], [ 1, %entry ]
+  %srcPtr.075.us = getelementptr inbounds i8, i8* %srcPtr.075.us.pn, i64 %extended
+
+  %0 = load i8, i8* %srcPtr.075.us, align 1, !tbaa !0
+  %1 = add i8 %0, 1
+  store i8 %1, i8* %srcPtr.075.us, align 1, !tbaa !0
+
+  %inc37.us = add nuw i32 %y.072.us, 1
+  %exitcond78 = icmp eq i32 %inc37.us, %sub
+  br i1 %exitcond78, label %for.cond.cleanup.loopexit, label %for.cond6.preheader.us
+
+for.cond.cleanup.loopexit:                        ; preds = %for.cond6.preheader.us
+  ret void
+}
+
+!0 = !{!1, !1, i64 0}
+!1 = !{!"omnipotent char", !2, i64 0}
+!2 = !{!"Simple C++ TBAA"}
+!3 = !{!4, !4, i64 0}
+!4 = !{!"float", !1, i64 0}