[BasicAA] Remove unnecessary known size requirement

[nikic]

Summary

This is tangentially related to D91383: After that patch, the offset/size conditions for the variable index case are the same as for the no-variable case, with one exception: If there are no variables and the base offset is negative, we also check that V2 doesn't have unknown size.

I believe this check is not necessary. It was introduced in https://github.com/llvm/llvm-project/commit/e3ac099726571b58333b0fee827831d420d24285, but the problem was really with incorrect location sizes being provided in the first place (wrt handling of negative offsets), which was fixed by https://github.com/llvm/llvm-project/commit/1726fc698ccb85fe4bb23c200a50f28b57fc53cb. Unknown location sizes are still required to be non-negative, so the second access cannot start below zero.

I've added a test using a store and a memset to illustrate a case where this makes a difference.

[nikic]

While I believe this change is correct, there are some incorrect AA uses that should be addressed first. There's one in MemorySSA wrt phi translation over decreasing cycles (https://reviews.llvm.org/D87778#inline-852179), and another in ParallelDSP (see test failure).

I think we need to clarify the meaning of LocationSize::unknown() in doc comments, and add a variant of it that means "unknown including negative sizes". The standard pattern for alias() queries is to simply pass both sizes as unknown if you want that behavior, but for getModRefInfo this trick doesn't work.

[asbirlea]

llvm/test/Analysis/MemorySSA/phi-translation.ll

397

This doesn't look right. The address loaded is modified in the loop.

[nikic]

Rebase over D91649, after which the correctness of this change should be more obvious. At this point in BasicAA, all LocationSizes only allow access after the base pointer.

The changes to MemorySSA tests are now gone.

[asbirlea]

Ack.

llvm/lib/Analysis/BasicAliasAnalysis.cpp

if (GEP1BaseOffset.sge(0)) {
       }
     } else {
       // We have the situation where:
       // +                +
       // | BaseOffset     |
       // ---------------->|
       // |-->V1Size       |-------> V2Size
       // GEP1             V2
-      // We need to know that V2Size is not unknown, otherwise we might have
-      // stripped a gep with negative index ('gep <ptr>, -1, ...).
-      if (V1Size != LocationSize::unknown() &&
-          V2Size != LocationSize::unknown()) {
+      if (V1Size != LocationSize::unknown()) {
         if ((-GEP1BaseOffset).ult(V1Size.getValue()))
           return PartialAlias;
         return NoAlias;
       }
     }
   }
 
   if (!DecompGEP1.VarIndices.empty()) {

llvm/test/Analysis/BasicAA/negoffset.ll

entry:
   store i32* %i.addr, i32** %p1
   %q2 = getelementptr inbounds %struct.X, %struct.X* %x, i32 0, i32 1
   store i32* %j.addr, i32** %q2
   %add.ptr = getelementptr inbounds i32*, i32** %q2, i32 %off
   %1 = load i32*, i32** %add.ptr
   %2 = load i32, i32* %1
   ret i32 %2
 }
+
+; CHECK-LABEL: Function: one_size_unknown:
+; CHECK: NoModRef:  Ptr: i8* %p.minus1  <->  call void @llvm.memset.p0i8.i32(i8* %p, i8 0, i32 %size, i1 false)
+define void @one_size_unknown(i8* %p, i32 %size) {
+  %p.minus1 = getelementptr inbounds i8, i8* %p, i32 -1
+  call void @llvm.memset.p0i8.i32(i8* %p, i8 0, i32 %size, i1 false)
+  ret void
+}
+
+declare void @llvm.memset.p0i8.i32(i8*, i8, i32, i1)

llvm/test/Analysis/MemorySSA/phi-translation.ll

 ; CHECK-NEXT:  %l.1 = load i32, i32* %p.1, align 4
 
 ; CHECK-LABEL: loop.1.latch:
 ; CHECK-NEXT:  ; 3 = MemoryPhi({loop.1.header,4},{storebb,2})
 
 ; CHECK-LABEL: storebb:
 ; CHECK-NEXT:  %iv.add2 = add nuw nsw i64 %iv, 2
 ; CHECK-NEXT:  %p.2 = getelementptr inbounds [32 x i32], [32 x i32]* %tmp, i64 0, i64 %iv.add2
-; CHECK-NEXT:  ; MemoryUse(4)
+; NOLIMIT-NEXT:; MemoryUse(1)
+; LIMIT-NEXT:  ; MemoryUse(4)
 ; CHECK-NEXT:  %l.2 = load i32, i32* %p.2, align 4
 ; CHECK-NEXT:  ; 2 = MemoryDef(4)
 ; CHECK-NEXT:  store i32 10, i32* %p.1, align 4
 entry:
   %tmp = alloca [32 x i32], align 16
   call void @init([32 x i32]* %tmp)
   br label %loop.1.header