[PATCH] D91482: [BasicAA] Remove unnecessary known size requirement

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: asbirlea, jdoerfert.
Herald added subscribers: llvm-commits, george.burgess.iv, hiraditya.
Herald added a project: LLVM.
nikic requested review of this revision.

This is tangentially related to D91383 <https://reviews.llvm.org/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. There is also a change in a MemorySSA test that deserves a careful look. However, the change looks correct to me there, as `p.2` is always after the stored location, even when considered across loop iterations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91482

Files:
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/test/Analysis/BasicAA/negoffset.ll
  llvm/test/Analysis/MemorySSA/phi-translation.ll


Index: llvm/test/Analysis/MemorySSA/phi-translation.ll
===================================================================
--- llvm/test/Analysis/MemorySSA/phi-translation.ll
+++ llvm/test/Analysis/MemorySSA/phi-translation.ll
@@ -394,7 +394,8 @@
 ; 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
Index: llvm/test/Analysis/BasicAA/negoffset.ll
===================================================================
--- llvm/test/Analysis/BasicAA/negoffset.ll
+++ llvm/test/Analysis/BasicAA/negoffset.ll
@@ -117,3 +117,13 @@
   %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)
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1380,10 +1380,7 @@
       // ---------------->|
       // |-->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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91482.305319.patch
Type: text/x-patch
Size: 2056 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201114/d8c731f7/attachment.bin>