[BasicAA] Handle wrapping of pointer arithmetic

[dtcxzyw]

This PR handles wrapping of pointer arithmetic conservatively, which is introduced by InstCombine (the behavior of C source code is well-defined).

Fixes #69096.

TBH, I think this workaround solution is ugly (and buggy?). Please let me know if you have a better one.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

This PR handles wrapping of pointer arithmetic conservatively, which is introduced by InstCombine (the behavior of C source code is well-defined).

Fixes #69096.

TBH, I think this workaround solution is ugly (and buggy?). Please let me know if you have a better one.

---

Full diff: https://github.com/llvm/llvm-project/pull/69116.diff

4 Files Affected:

• (modified) llvm/lib/Analysis/BasicAliasAnalysis.cpp (+7-2)
• (modified) llvm/test/Analysis/BasicAA/assume-index-positive.ll (+1-1)
• (modified) llvm/test/Analysis/BasicAA/gep-modulo.ll (+2-2)
• (added) llvm/test/Analysis/BasicAA/pr69096.ll (+30)

diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index c162b8f6edc1905..a19524b527682f3 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -417,7 +417,8 @@ static LinearExpression GetLinearExpression(
                                 Depth + 1, AC, DT);
         E.Offset <<= RHS.getLimitedValue();
         E.Scale <<= RHS.getLimitedValue();
-        E.IsNSW &= NSW;
+        // The nsw flag has different semantics for shift and mul.
+        E.IsNSW = false;
         break;
       }
       return E;
@@ -1184,7 +1185,11 @@ AliasResult BasicAAResult::aliasGEP(
   APInt ModOffset = DecompGEP1.Offset.srem(GCD);
   if (ModOffset.isNegative())
     ModOffset += GCD; // We want mod, not rem.
-  if (ModOffset.uge(V2Size.getValue()) &&
+  unsigned PtrBW =
+      DL.getPointerSizeInBits(GEP1->getType()->getPointerAddressSpace());
+  if ((GCD.isPowerOf2() || !OffsetRange.sextOrTrunc(PtrBW).isFullSet() ||
+       (*DecompGEP1.InBounds && DecompGEP2.InBounds == true)) &&
+      ModOffset.uge(V2Size.getValue()) &&
       (GCD - ModOffset).uge(V1Size.getValue()))
     return AliasResult::NoAlias;
 
diff --git a/llvm/test/Analysis/BasicAA/assume-index-positive.ll b/llvm/test/Analysis/BasicAA/assume-index-positive.ll
index 42014dfe0264218..d5a7b540266b6cf 100644
--- a/llvm/test/Analysis/BasicAA/assume-index-positive.ll
+++ b/llvm/test/Analysis/BasicAA/assume-index-positive.ll
@@ -116,7 +116,7 @@ define void @shl_of_non_negative(ptr %ptr, i64 %a) {
 define void @shl_nsw_of_non_negative(ptr %ptr, i64 %a) {
 ; CHECK-LABEL: Function: shl_nsw_of_non_negative
 ; CHECK: NoAlias: i8* %ptr.a, i8* %ptr.neg
-; CHECK: NoAlias: i8* %ptr.neg, i8* %ptr.shl
+; CHECK: MayAlias: i8* %ptr.neg, i8* %ptr.shl
 ;
   %a.cmp = icmp sge i64 %a, 0
   call void @llvm.assume(i1 %a.cmp)
diff --git a/llvm/test/Analysis/BasicAA/gep-modulo.ll b/llvm/test/Analysis/BasicAA/gep-modulo.ll
index dcbc0a9090852e4..7c8c1f3b3ee34d2 100644
--- a/llvm/test/Analysis/BasicAA/gep-modulo.ll
+++ b/llvm/test/Analysis/BasicAA/gep-modulo.ll
@@ -90,7 +90,7 @@ define void @nuw_nsw_mul_sub_i64(ptr %ptr, i64 %idx) {
 ; CHECK-LABEL: Function: nuw_nsw_mul_sub_i64: 3 pointers, 0 call sites
 ; CHECK-NEXT:    MayAlias:  i8* %gep.idx, [16 x i8]* %ptr
 ; CHECK-NEXT:    PartialAlias (off -3): i8* %gep.3, [16 x i8]* %ptr
-; CHECK-NEXT:    NoAlias:  i8* %gep.3, i8* %gep.idx
+; CHECK-NEXT:    MayAlias:  i8* %gep.3, i8* %gep.idx
 ;
   load [16 x i8], ptr %ptr
   %mul = mul nuw nsw i64 %idx, 5
@@ -106,7 +106,7 @@ define void @only_nsw_mul_sub_i64(ptr %ptr, i64 %idx) {
 ; CHECK-LABEL: Function: only_nsw_mul_sub_i64: 3 pointers, 0 call sites
 ; CHECK-NEXT:    MayAlias:  i8* %gep.idx, [16 x i8]* %ptr
 ; CHECK-NEXT:    PartialAlias (off -3): i8* %gep.3, [16 x i8]* %ptr
-; CHECK-NEXT:    NoAlias:  i8* %gep.3, i8* %gep.idx
+; CHECK-NEXT:    MayAlias:  i8* %gep.3, i8* %gep.idx
 ;
   load [16 x i8], ptr %ptr
   %mul = mul nsw i64 %idx, 5
diff --git a/llvm/test/Analysis/BasicAA/pr69096.ll b/llvm/test/Analysis/BasicAA/pr69096.ll
new file mode 100644
index 000000000000000..a55cfaff45a2bd9
--- /dev/null
+++ b/llvm/test/Analysis/BasicAA/pr69096.ll
@@ -0,0 +1,30 @@
+; RUN: opt %s -aa-pipeline=basic-aa -passes=aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
+
+target datalayout = "p:64:64:64"
+
+; CHECK-LABEL: Function: pr69096
+; CHECK: MayAlias:     i8* %p, i16* %scevgep.i
+
+define i32 @pr69096(i16 %a, ptr %p) {
+entry:
+  %0 = load i8, ptr %p, align 2
+  %dec.i = add i8 %0, -1
+  %cmp636.i = icmp eq i16 %a, -1
+  br i1 %cmp636.i, label %for.cond2.for.inc29_crit_edge.i, label %n.exit
+
+for.cond2.for.inc29_crit_edge.i:
+  %conv3.i = zext i16 %a to i64
+  %sub.i.i = shl i64 %conv3.i, 56
+  %sub21.i = shl nuw nsw i64 %conv3.i, 2
+  %1 = getelementptr i8, ptr %p, i64 %sub21.i
+  %2 = getelementptr i8, ptr %1, i64 -262140
+  %3 = getelementptr i8, ptr %2, i64 %sub.i.i
+  %scevgep.i = getelementptr i8, ptr %3, i64 72057594037927936
+  store i16 1285, ptr %scevgep.i, align 2
+  br label %n.exit
+
+n.exit:
+  %4 = load i8, ptr %p, align 2
+  %conv = sext i8 %4 to i32
+  ret i32 %conv
+}

[nikic]

This is conceptually right, but you should keep the nsw flag if you know that either RHS is a constant other than bitwidth-1 or the LHS is a constant other than -1.

Please split off this part of the patch with a minimal test that demonstrates that the current behavior is wrong in this specific edge case.

[nikic]

Well, I guess LHS being a constant isn't relevant in this code, so just checking the RHS is enough.

[dtcxzyw]

Sorry for my misunderstanding. shl nsw %a, c1 is equivalent to mul nsw %a, (1<<c1).
Alive2: https://alive2.llvm.org/ce/z/clpceU
I have found the root cause of this issue. I will propose a fix soon.

[nikic]

I don't understand what you're trying to do here, but it's almost certainly not right. Please minimize the test case and provide an analysis of why it produced an incorrect result.