[llvm] [BasicAA] Use nuw attribute of GEPs (PR #98608)

Hari Limaye via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 14 01:12:18 PDT 2024


https://github.com/hazzlim updated https://github.com/llvm/llvm-project/pull/98608

>From be98e95588b9c8cdd095cc25df1a0f8c417e3ca4 Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Wed, 26 Jun 2024 15:09:35 +0000
Subject: [PATCH 1/4] [BasicAA] Use nuw attribute of GEPs

Use the nuw attribute of GEPs to prove that pointers do not alias, in
cases matching the following:

   +                +                     +
   | BaseOffset     |   +<nuw> Indices    |
   ---------------->|-------------------->|
   |-->VLeftSize    |                     |------->VRightSize
  LHS                                    RHS

If the difference between pointers is Offset +<nuw> Indices (the variable
indices all come from nuw GEPs) then we know that the addition does not
wrap the pointer index type (add nuw) and the constant Offset is a lower
bound on the distance between the pointers. We can then prove NoAlias via
Offset u>= VLeftSize.
---
 llvm/lib/Analysis/BasicAliasAnalysis.cpp    |  45 +++++--
 llvm/test/Analysis/BasicAA/gep-nuw-alias.ll | 142 ++++++++++++++++++++
 2 files changed, 177 insertions(+), 10 deletions(-)
 create mode 100644 llvm/test/Analysis/BasicAA/gep-nuw-alias.ll

diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 7bfb23e14aaa79..c531d266a61d6a 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -552,9 +552,9 @@ struct BasicAAResult::DecomposedGEP {
   APInt Offset;
   // Scaled variable (non-constant) indices.
   SmallVector<VariableGEPIndex, 4> VarIndices;
-  // Are all operations inbounds GEPs or non-indexing operations?
+  // Nowrap flags common to all GEP operations involved in expression.
   // (std::nullopt iff expression doesn't involve any geps)
-  std::optional<bool> InBounds;
+  std::optional<GEPNoWrapFlags> NWFlags;
 
   void dump() const {
     print(dbgs());
@@ -641,12 +641,11 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
       return Decomposed;
     }
 
-    // Track whether we've seen at least one in bounds gep, and if so, whether
-    // all geps parsed were in bounds.
-    if (Decomposed.InBounds == std::nullopt)
-      Decomposed.InBounds = GEPOp->isInBounds();
-    else if (!GEPOp->isInBounds())
-      Decomposed.InBounds = false;
+    // Track the common nowrap flags for all GEPs we see.
+    if (Decomposed.NWFlags == std::nullopt)
+      Decomposed.NWFlags = GEPOp->getNoWrapFlags();
+    else
+      *Decomposed.NWFlags &= GEPOp->getNoWrapFlags();
 
     assert(GEPOp->getSourceElementType()->isSized() && "GEP must be sized");
 
@@ -1117,7 +1116,7 @@ AliasResult BasicAAResult::aliasGEP(
   // for the two to alias, then we can assume noalias.
   // TODO: Remove !isScalable() once BasicAA fully support scalable location
   // size
-  if (*DecompGEP1.InBounds && DecompGEP1.VarIndices.empty() &&
+  if (DecompGEP1.NWFlags->isInBounds() && DecompGEP1.VarIndices.empty() &&
       V2Size.hasValue() && !V2Size.isScalable() &&
       DecompGEP1.Offset.sge(V2Size.getValue()) &&
       isBaseOfObject(DecompGEP2.Base))
@@ -1125,7 +1124,7 @@ AliasResult BasicAAResult::aliasGEP(
 
   if (isa<GEPOperator>(V2)) {
     // Symmetric case to above.
-    if (*DecompGEP2.InBounds && DecompGEP1.VarIndices.empty() &&
+    if (DecompGEP2.NWFlags->isInBounds() && DecompGEP1.VarIndices.empty() &&
         V1Size.hasValue() && !V1Size.isScalable() &&
         DecompGEP1.Offset.sle(-V1Size.getValue()) &&
         isBaseOfObject(DecompGEP1.Base))
@@ -1239,6 +1238,32 @@ AliasResult BasicAAResult::aliasGEP(
     }
   }
 
+  // If the difference between pointers is Offset +<nuw> Indices (the variable
+  // indices all come from nuw GEPs) then we know that the addition does not
+  // wrap the pointer index type (add nuw) and the constant Offset is a lower
+  // bound on the distance between the pointers. We can then prove NoAlias via
+  // Offset u>= VLeftSize.
+  //    +                +                     +
+  //    | BaseOffset     |   +<nuw> Indices    |
+  //    ---------------->|-------------------->|
+  //    |-->VLeftSize    |                     |-------> VRightSize
+  //   LHS                                    RHS
+  if (!DecompGEP1.VarIndices.empty() &&
+      llvm::all_of(DecompGEP1.VarIndices, [&](const VariableGEPIndex &V) {
+        return V.IsNegated == DecompGEP1.VarIndices.front().IsNegated;
+      })) {
+    APInt Off = DecompGEP1.Offset;
+    bool Swapped = Off.isNegative();
+    LocationSize VLeftSize = Swapped ? V1Size : V2Size;
+    DecomposedGEP &DecompRight = Swapped ? DecompGEP2 : DecompGEP1;
+
+    bool IndicesFromRight = DecompGEP1.VarIndices.front().IsNegated == Swapped;
+    if (IndicesFromRight && DecompRight.NWFlags->hasNoUnsignedWrap())
+      if (!VLeftSize.isScalable() && VLeftSize.hasValue() &&
+          Off.abs().uge(VLeftSize.getValue()))
+        return AliasResult::NoAlias;
+  }
+
   // Bail on analysing scalable LocationSize
   if (V1Size.isScalable() || V2Size.isScalable())
     return AliasResult::MayAlias;
diff --git a/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll b/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
new file mode 100644
index 00000000000000..79899d60640d6e
--- /dev/null
+++ b/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
@@ -0,0 +1,142 @@
+; RUN: opt < %s -aa-pipeline=basic-aa -passes=aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; CHECK-LABEL: test_no_lower_bound
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+define void @test_no_lower_bound(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p, i64 4
+  %b = getelementptr nuw i8, ptr %p, i64 %i
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: test_lower_bound_lt_size
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+define void @test_lower_bound_lt_size(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p
+  %add = getelementptr nuw i8, ptr %p, i64 2
+  %b = getelementptr nuw i8, ptr %add, i64 %i
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: test_lower_bound_ge_size
+;
+; CHECK-DAG: NoAlias: i32* %a, i32* %b
+define void @test_lower_bound_ge_size(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %b = getelementptr nuw i8, ptr %add, i64 %i
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: test_not_all_nuw
+;
+; If part of the addressing is done with non-nuw GEPs, we can't use properties
+; implied by the last GEP with the whole offset. In this case, the calculation
+; of %add (%p + 4) could wrap the pointer index type, such that %add +<nuw> %i
+; could still alias with %p.
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+define void @test_not_all_nuw(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p
+  %add = getelementptr i8, ptr %p, i64 4
+  %b = getelementptr nuw i8, ptr %add, i64 %i
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: test_multi_step_not_all_nuw
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+define void @test_multi_step_not_all_nuw(ptr %p, i64 %i, i64 %j, i64 %k) {
+  %a = getelementptr i8, ptr %p
+  %add = getelementptr i8, ptr %p, i64 4
+  %step1 = getelementptr i8, ptr %add, i64 %i
+  %step2 = getelementptr i8, ptr %step1, i64 %j
+  %b = getelementptr nuw i8, ptr %step2, i64 %k
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: test_multi_step_all_nuw
+;
+; CHECK-DAG: NoAlias: i32* %a, i32* %b
+define void @test_multi_step_all_nuw(ptr %p, i64 %i, i64 %j, i64 %k) {
+  %a = getelementptr i8, ptr %p
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %step1 = getelementptr nuw i8, ptr %add, i64 %i
+  %step2 = getelementptr nuw i8, ptr %step1, i64 %j
+  %b = getelementptr nuw i8, ptr %step2, i64 %k
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+%struct = type { i64, [2 x i32], i64 }
+
+; CHECK-LABEL: test_struct_no_nuw
+;
+; The array access may alias with the struct elements before and after, because
+; we cannot prove that (%arr + %i) does not alias with the base pointer %p.
+;
+; CHECK-DAG: MayAlias: i32* %arrayidx, i64* %st
+; CHECK-DAG: NoAlias: i64* %after, i64* %st
+; CHECK-DAG: MayAlias: i64* %after, i32* %arrayidx
+
+define void @test_struct_no_nuw(ptr %st, i64 %i) {
+  %arr = getelementptr i8, ptr %st, i64 8
+  %arrayidx = getelementptr [2 x i32], ptr %arr, i64 0, i64 %i
+  %after = getelementptr i8, ptr %st, i64 16
+
+  load i64, ptr %st
+  load i32, ptr %arrayidx
+  load i64, ptr %after
+
+  ret void
+}
+
+; CHECK-LABEL: test_struct_nuw
+;
+; We can prove that the array access does not alias with struct element before,
+; because we can prove that (%arr +<nuw> %i) does not wrap the pointer index
+; type (add nuw). The array access may still alias with the struct element
+; after, as the add nuw property does not preclude this.
+;
+; CHECK-DAG: NoAlias: i32* %arrayidx, i64* %st
+; CHECK-DAG: NoAlias: i64* %after, i64* %st
+; CHECK-DAG: MayAlias: i64* %after, i32* %arrayidx
+
+define void @test_struct_nuw(ptr %st, i64 %i) {
+  %arr = getelementptr nuw i8, ptr %st, i64 8
+  %arrayidx = getelementptr nuw [2 x i32], ptr %arr, i64 0, i64 %i
+  %after = getelementptr nuw i8, ptr %st, i64 16
+
+  load i64, ptr %st
+  load i32, ptr %arrayidx
+  load i64, ptr %after
+
+  ret void
+}
+

>From f2fad1bc9f33a7cb1a49682c0c4fb991e63bea44 Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Mon, 15 Jul 2024 08:24:59 +0000
Subject: [PATCH 2/4] Reorder checks (Addressing review comment)

---
 llvm/lib/Analysis/BasicAliasAnalysis.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index c531d266a61d6a..36402d06537a97 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1259,7 +1259,7 @@ AliasResult BasicAAResult::aliasGEP(
 
     bool IndicesFromRight = DecompGEP1.VarIndices.front().IsNegated == Swapped;
     if (IndicesFromRight && DecompRight.NWFlags->hasNoUnsignedWrap())
-      if (!VLeftSize.isScalable() && VLeftSize.hasValue() &&
+      if (VLeftSize.hasValue() && !VLeftSize.isScalable() &&
           Off.abs().uge(VLeftSize.getValue()))
         return AliasResult::NoAlias;
   }

>From 846411b4dba7326aa5a5fab28ed4e14a770bc9ac Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Thu, 8 Aug 2024 23:17:26 +0000
Subject: [PATCH 3/4] Drop nuw flag in some cases when subtracting GEPs

---
 .../llvm/Analysis/BasicAliasAnalysis.h        |  3 +-
 llvm/lib/Analysis/BasicAliasAnalysis.cpp      | 41 ++++++---
 llvm/test/Analysis/BasicAA/gep-nuw-alias.ll   | 92 +++++++++++++++++++
 3 files changed, 121 insertions(+), 15 deletions(-)

diff --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
index 7eca82729430dd..801d55d09731c5 100644
--- a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
@@ -122,8 +122,7 @@ class BasicAAResult : public AAResultBase {
   bool isValueEqualInPotentialCycles(const Value *V1, const Value *V2,
                                      const AAQueryInfo &AAQI);
 
-  void subtractDecomposedGEPs(DecomposedGEP &DestGEP,
-                              const DecomposedGEP &SrcGEP,
+  void subtractDecomposedGEPs(DecomposedGEP &DestGEP, DecomposedGEP &SrcGEP,
                               const AAQueryInfo &AAQI);
 
   AliasResult aliasGEP(const GEPOperator *V1, LocationSize V1Size,
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 36402d06537a97..95b46cac81a05e 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -527,6 +527,8 @@ struct VariableGEPIndex {
     return Scale == Other.Scale;
   }
 
+  bool isSubtracted() const { return IsNegated || Scale.isNegative(); }
+
   void dump() const {
     print(dbgs());
     dbgs() << "\n";
@@ -561,8 +563,8 @@ struct BasicAAResult::DecomposedGEP {
     dbgs() << "\n";
   }
   void print(raw_ostream &OS) const {
-    OS << "(DecomposedGEP Base=" << Base->getName()
-       << ", Offset=" << Offset
+    OS << "(DecomposedGEP Base=" << Base->getName() << ", Offset=" << Offset
+       << ", nuw=" << ((NWFlags && NWFlags->hasNoUnsignedWrap()) ? "1" : "0")
        << ", VarIndices=[";
     for (size_t i = 0; i < VarIndices.size(); i++) {
       if (i != 0)
@@ -1238,11 +1240,10 @@ AliasResult BasicAAResult::aliasGEP(
     }
   }
 
-  // If the difference between pointers is Offset +<nuw> Indices (the variable
-  // indices all come from nuw GEPs) then we know that the addition does not
-  // wrap the pointer index type (add nuw) and the constant Offset is a lower
-  // bound on the distance between the pointers. We can then prove NoAlias via
-  // Offset u>= VLeftSize.
+  // If the difference between pointers is Offset +<nuw> Indices then we know
+  // that the addition does not wrap the pointer index type (add nuw) and the
+  // constant Offset is a lower bound on the distance between the pointers. We
+  // can then prove NoAlias via Offset u>= VLeftSize.
   //    +                +                     +
   //    | BaseOffset     |   +<nuw> Indices    |
   //    ---------------->|-------------------->|
@@ -1250,15 +1251,15 @@ AliasResult BasicAAResult::aliasGEP(
   //   LHS                                    RHS
   if (!DecompGEP1.VarIndices.empty() &&
       llvm::all_of(DecompGEP1.VarIndices, [&](const VariableGEPIndex &V) {
-        return V.IsNegated == DecompGEP1.VarIndices.front().IsNegated;
+        return V.isSubtracted() == DecompGEP1.VarIndices.front().isSubtracted();
       })) {
-    APInt Off = DecompGEP1.Offset;
+    const APInt &Off = DecompGEP1.Offset;
     bool Swapped = Off.isNegative();
     LocationSize VLeftSize = Swapped ? V1Size : V2Size;
-    DecomposedGEP &DecompRight = Swapped ? DecompGEP2 : DecompGEP1;
+    const DecomposedGEP &DecompRight = Swapped ? DecompGEP2 : DecompGEP1;
 
-    bool IndicesFromRight = DecompGEP1.VarIndices.front().IsNegated == Swapped;
-    if (IndicesFromRight && DecompRight.NWFlags->hasNoUnsignedWrap())
+    bool IndicesAdded = DecompGEP1.VarIndices.front().isSubtracted() == Swapped;
+    if (IndicesAdded && DecompRight.NWFlags->hasNoUnsignedWrap())
       if (VLeftSize.hasValue() && !VLeftSize.isScalable() &&
           Off.abs().uge(VLeftSize.getValue()))
         return AliasResult::NoAlias;
@@ -1866,8 +1867,15 @@ bool BasicAAResult::isValueEqualInPotentialCycles(const Value *V,
 
 /// Computes the symbolic difference between two de-composed GEPs.
 void BasicAAResult::subtractDecomposedGEPs(DecomposedGEP &DestGEP,
-                                           const DecomposedGEP &SrcGEP,
+                                           DecomposedGEP &SrcGEP,
                                            const AAQueryInfo &AAQI) {
+  // Drop nuw flag from GEP if subtraction of constant offsets overflows in an
+  // unsigned sense.
+  if (DestGEP.Offset.ult(SrcGEP.Offset))
+    DestGEP.NWFlags = DestGEP.NWFlags->withoutNoUnsignedWrap();
+  else if (SrcGEP.Offset.ult(DestGEP.Offset) && SrcGEP.NWFlags)
+    SrcGEP.NWFlags = SrcGEP.NWFlags->withoutNoUnsignedWrap();
+
   DestGEP.Offset -= SrcGEP.Offset;
   for (const VariableGEPIndex &Src : SrcGEP.VarIndices) {
     // Find V in Dest.  This is N^2, but pointer indices almost never have more
@@ -1890,6 +1898,13 @@ void BasicAAResult::subtractDecomposedGEPs(DecomposedGEP &DestGEP,
       // If we found it, subtract off Scale V's from the entry in Dest.  If it
       // goes to zero, remove the entry.
       if (Dest.Scale != Src.Scale) {
+        // Drop nuw flag from GEP if subtraction of V's Scale overflows in an
+        // unsigned sense.
+        if (Dest.Scale.ult(Src.Scale))
+          DestGEP.NWFlags = DestGEP.NWFlags->withoutNoUnsignedWrap();
+        else if (SrcGEP.NWFlags)
+          SrcGEP.NWFlags = SrcGEP.NWFlags->withoutNoUnsignedWrap();
+
         Dest.Scale -= Src.Scale;
         Dest.IsNSW = false;
       } else {
diff --git a/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll b/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
index 79899d60640d6e..fc888f5e29eded 100644
--- a/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
+++ b/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
@@ -140,3 +140,95 @@ define void @test_struct_nuw(ptr %st, i64 %i) {
   ret void
 }
 
+; CHECK-LABEL: constant_offset_overflow
+;
+; If subtraction of constant offsets could overflow in an unsigned sense, we
+; cannot prove the lower bound between the GEPs and so they may still alias.
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+
+define void @constant_offset_overflow(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p, i64 -8
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %b = getelementptr nuw i8, ptr %add, i64 %i
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: constant_offset_overflow_rev_order
+;
+; Same as above test, just verifying correct behaviour when GEPs encountered
+; in the reverse order;
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+
+define void @constant_offset_overflow_rev_order(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p, i64 -8
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %b = getelementptr nuw i8, ptr %add, i64 %i
+
+  load i32, ptr %b
+  load i32, ptr %a
+
+  ret void
+}
+
+; CHECK-LABEL: equal_var_idx_noalias
+;
+; If GEPs have equal variable indices, we can prove NoAlias when the Scale of
+; the RHS GEP is greater, as in this scenario the constant lower bound holds.
+;
+; CHECK-DAG: NoAlias: i32* %a, i32* %b
+
+define void @equal_var_idx_noalias(ptr %p, i64 %i) {
+  %a = getelementptr i8, ptr %p, i64 %i
+
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %b = getelementptr nuw i16, ptr %add, i64 %i
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}
+
+; CHECK-LABEL: equal_var_idx_alias
+;
+; If GEPs have equal variable indices, we cannot prove NoAlias when the Scale of
+; the RHS GEP is ult Scale of the LHS GEP.
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+
+define void @equal_var_idx_alias(ptr %p, i64 %i) {
+  %a = getelementptr i32, ptr %p, i64 %i
+
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %b = getelementptr nuw i16, ptr %add, i64 %i
+
+  load i32, ptr %b
+  load i32, ptr %a
+
+  ret void
+}
+
+; CHECK-LABEL: both_var_idx
+;
+; If the RHS GEP has unmatched variable indices, we cannot prove a constant
+; lower bound between GEPs.
+;
+; CHECK-DAG: MayAlias: i32* %a, i32* %b
+
+define void @both_var_idx(ptr %p, i64 %i, i64 %j) {
+  %a = getelementptr i8, ptr %p, i64 %i
+
+  %add = getelementptr nuw i8, ptr %p, i64 4
+  %b = getelementptr nuw i8, ptr %add, i64 %j
+
+  load i32, ptr %a
+  load i32, ptr %b
+
+  ret void
+}

>From 76a7b1b6fe8f1391399b16df7064d23c45a3cd0c Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Tue, 13 Aug 2024 22:54:42 +0000
Subject: [PATCH 4/4] Swap order of GEPs to simplify things

---
 llvm/lib/Analysis/BasicAliasAnalysis.cpp    | 54 +++++++++------------
 llvm/test/Analysis/BasicAA/gep-nuw-alias.ll | 18 -------
 2 files changed, 22 insertions(+), 50 deletions(-)

diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 95b46cac81a05e..7576d647e64732 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1110,6 +1110,13 @@ AliasResult BasicAAResult::aliasGEP(
   if (DecompGEP1.Base == GEP1 && DecompGEP2.Base == V2)
     return AliasResult::MayAlias;
 
+  // Swap GEP1 and GEP2 if GEP2 has more variable indices.
+  if (DecompGEP1.VarIndices.size() < DecompGEP2.VarIndices.size()) {
+    std::swap(DecompGEP1, DecompGEP2);
+    std::swap(V1Size, V2Size);
+    std::swap(UnderlyingV1, UnderlyingV2);
+  }
+
   // Subtract the GEP2 pointer from the GEP1 pointer to find out their
   // symbolic difference.
   subtractDecomposedGEPs(DecompGEP1, DecompGEP2, AAQI);
@@ -1118,20 +1125,18 @@ AliasResult BasicAAResult::aliasGEP(
   // for the two to alias, then we can assume noalias.
   // TODO: Remove !isScalable() once BasicAA fully support scalable location
   // size
-  if (DecompGEP1.NWFlags->isInBounds() && DecompGEP1.VarIndices.empty() &&
-      V2Size.hasValue() && !V2Size.isScalable() &&
-      DecompGEP1.Offset.sge(V2Size.getValue()) &&
+  if (DecompGEP1.NWFlags && DecompGEP1.NWFlags->isInBounds() &&
+      DecompGEP1.VarIndices.empty() && V2Size.hasValue() &&
+      !V2Size.isScalable() && DecompGEP1.Offset.sge(V2Size.getValue()) &&
       isBaseOfObject(DecompGEP2.Base))
     return AliasResult::NoAlias;
 
-  if (isa<GEPOperator>(V2)) {
-    // Symmetric case to above.
-    if (DecompGEP2.NWFlags->isInBounds() && DecompGEP1.VarIndices.empty() &&
-        V1Size.hasValue() && !V1Size.isScalable() &&
-        DecompGEP1.Offset.sle(-V1Size.getValue()) &&
-        isBaseOfObject(DecompGEP1.Base))
-      return AliasResult::NoAlias;
-  }
+  // Symmetric case to above.
+  if (DecompGEP2.NWFlags && DecompGEP2.NWFlags->isInBounds() &&
+      DecompGEP1.VarIndices.empty() && V1Size.hasValue() &&
+      !V1Size.isScalable() && DecompGEP1.Offset.sle(-V1Size.getValue()) &&
+      isBaseOfObject(DecompGEP1.Base))
+    return AliasResult::NoAlias;
 
   // For GEPs with identical offsets, we can preserve the size and AAInfo
   // when performing the alias check on the underlying objects.
@@ -1160,8 +1165,6 @@ AliasResult BasicAAResult::aliasGEP(
     APInt &Off = DecompGEP1.Offset;
 
     // Initialize for Off >= 0 (V2 <= GEP1) case.
-    const Value *LeftPtr = V2;
-    const Value *RightPtr = GEP1;
     LocationSize VLeftSize = V2Size;
     LocationSize VRightSize = V1Size;
     const bool Swapped = Off.isNegative();
@@ -1173,7 +1176,6 @@ AliasResult BasicAAResult::aliasGEP(
       // ---------------->|
       // |-->V1Size       |-------> V2Size
       // GEP1             V2
-      std::swap(LeftPtr, RightPtr);
       std::swap(VLeftSize, VRightSize);
       Off = -Off;
     }
@@ -1250,20 +1252,9 @@ AliasResult BasicAAResult::aliasGEP(
   //    |-->VLeftSize    |                     |-------> VRightSize
   //   LHS                                    RHS
   if (!DecompGEP1.VarIndices.empty() &&
-      llvm::all_of(DecompGEP1.VarIndices, [&](const VariableGEPIndex &V) {
-        return V.isSubtracted() == DecompGEP1.VarIndices.front().isSubtracted();
-      })) {
-    const APInt &Off = DecompGEP1.Offset;
-    bool Swapped = Off.isNegative();
-    LocationSize VLeftSize = Swapped ? V1Size : V2Size;
-    const DecomposedGEP &DecompRight = Swapped ? DecompGEP2 : DecompGEP1;
-
-    bool IndicesAdded = DecompGEP1.VarIndices.front().isSubtracted() == Swapped;
-    if (IndicesAdded && DecompRight.NWFlags->hasNoUnsignedWrap())
-      if (VLeftSize.hasValue() && !VLeftSize.isScalable() &&
-          Off.abs().uge(VLeftSize.getValue()))
-        return AliasResult::NoAlias;
-  }
+      DecompGEP1.NWFlags->hasNoUnsignedWrap() && V2Size.hasValue() &&
+      !V2Size.isScalable() && DecompGEP1.Offset.sge(V2Size.getValue()))
+    return AliasResult::NoAlias;
 
   // Bail on analysing scalable LocationSize
   if (V1Size.isScalable() || V2Size.isScalable())
@@ -1873,8 +1864,6 @@ void BasicAAResult::subtractDecomposedGEPs(DecomposedGEP &DestGEP,
   // unsigned sense.
   if (DestGEP.Offset.ult(SrcGEP.Offset))
     DestGEP.NWFlags = DestGEP.NWFlags->withoutNoUnsignedWrap();
-  else if (SrcGEP.Offset.ult(DestGEP.Offset) && SrcGEP.NWFlags)
-    SrcGEP.NWFlags = SrcGEP.NWFlags->withoutNoUnsignedWrap();
 
   DestGEP.Offset -= SrcGEP.Offset;
   for (const VariableGEPIndex &Src : SrcGEP.VarIndices) {
@@ -1902,8 +1891,6 @@ void BasicAAResult::subtractDecomposedGEPs(DecomposedGEP &DestGEP,
         // unsigned sense.
         if (Dest.Scale.ult(Src.Scale))
           DestGEP.NWFlags = DestGEP.NWFlags->withoutNoUnsignedWrap();
-        else if (SrcGEP.NWFlags)
-          SrcGEP.NWFlags = SrcGEP.NWFlags->withoutNoUnsignedWrap();
 
         Dest.Scale -= Src.Scale;
         Dest.IsNSW = false;
@@ -1919,6 +1906,9 @@ void BasicAAResult::subtractDecomposedGEPs(DecomposedGEP &DestGEP,
       VariableGEPIndex Entry = {Src.Val, Src.Scale, Src.CxtI, Src.IsNSW,
                                 /* IsNegated */ true};
       DestGEP.VarIndices.push_back(Entry);
+
+      // Drop nuw flag when we have unconsumed variable indices from SrcGEP.
+      DestGEP.NWFlags = DestGEP.NWFlags->withoutNoUnsignedWrap();
     }
   }
 }
diff --git a/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll b/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
index fc888f5e29eded..d3201ad8f74254 100644
--- a/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
+++ b/llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
@@ -158,24 +158,6 @@ define void @constant_offset_overflow(ptr %p, i64 %i) {
   ret void
 }
 
-; CHECK-LABEL: constant_offset_overflow_rev_order
-;
-; Same as above test, just verifying correct behaviour when GEPs encountered
-; in the reverse order;
-;
-; CHECK-DAG: MayAlias: i32* %a, i32* %b
-
-define void @constant_offset_overflow_rev_order(ptr %p, i64 %i) {
-  %a = getelementptr i8, ptr %p, i64 -8
-  %add = getelementptr nuw i8, ptr %p, i64 4
-  %b = getelementptr nuw i8, ptr %add, i64 %i
-
-  load i32, ptr %b
-  load i32, ptr %a
-
-  ret void
-}
-
 ; CHECK-LABEL: equal_var_idx_noalias
 ;
 ; If GEPs have equal variable indices, we can prove NoAlias when the Scale of



More information about the llvm-commits mailing list