Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Apr 29, 2025

Similar to 1b7ef6a, add a check to only set MinAbsVarIndex if abs(Scale*V0) and abs((-Scale)*V1) won't wrap. In the absence of IsNSW, try to use the bitwidths of the original V and Scale to rule out wrapping

topperc added 2 commits April 28, 2025 22:14
… >= abs(Scale)

Similar to 1b7ef6a, add a check to
only set MinAbsVarIndex if abs(Scale*V0) and abs((-Scale)*V1) won't
wrap. In the absence of IsNSW, try to use the bitwidths of the
original V and Scale to rule out wrapping
@topperc topperc requested a review from fhahn April 29, 2025 05:15
@topperc topperc requested a review from nikic as a code owner April 29, 2025 05:15
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Apr 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 29, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Craig Topper (topperc)

Changes

Similar to 1b7ef6a, add a check to
only set MinAbsVarIndex if abs(Scale*V0) and abs((-Scale)*V1) won't
wrap. In the absence of IsNSW, try to use the bitwidths of the
original V and Scale to rule out wrapping


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/BasicAliasAnalysis.cpp (+19-17)
  • (modified) llvm/test/Analysis/BasicAA/gep-modulo.ll (+14)
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index cbec13c7808be..03977551afadd 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1301,6 +1301,23 @@ AliasResult BasicAAResult::aliasGEP(
   if (Range1.intersectWith(Range2).isEmptySet())
     return AliasResult::NoAlias;
 
+  // Check if abs(V*Scale) >= abs(Scale) holds in the presence of
+  // potentially wrapping math.
+  auto MultiplyByScaleNoWrap = [](const VariableGEPIndex &Var) {
+    if (Var.IsNSW)
+      return true;
+
+    int ValOrigBW = Var.Val.V->getType()->getPrimitiveSizeInBits();
+    // If Scale is small enough so that abs(V*Scale) >= abs(Scale) holds.
+    // The max value of abs(V) is 2^ValOrigBW - 1. Multiplying with a
+    // constant smaller than 2^(bitwidth(Val) - ValOrigBW) won't wrap.
+    int MaxScaleValueBW = Var.Val.getBitWidth() - ValOrigBW;
+    if (MaxScaleValueBW <= 0)
+      return false;
+    return Var.Scale.ule(
+        APInt::getMaxValue(MaxScaleValueBW).zext(Var.Scale.getBitWidth()));
+  };
+
   // Try to determine the range of values for VarIndex such that
   // VarIndex <= -MinAbsVarIndex || MinAbsVarIndex <= VarIndex.
   std::optional<APInt> MinAbsVarIndex;
@@ -1309,22 +1326,6 @@ AliasResult BasicAAResult::aliasGEP(
     const VariableGEPIndex &Var = DecompGEP1.VarIndices[0];
     if (Var.Val.TruncBits == 0 &&
         isKnownNonZero(Var.Val.V, SimplifyQuery(DL, DT, &AC, Var.CxtI))) {
-      // Check if abs(V*Scale) >= abs(Scale) holds in the presence of
-      // potentially wrapping math.
-      auto MultiplyByScaleNoWrap = [](const VariableGEPIndex &Var) {
-        if (Var.IsNSW)
-          return true;
-
-        int ValOrigBW = Var.Val.V->getType()->getPrimitiveSizeInBits();
-        // If Scale is small enough so that abs(V*Scale) >= abs(Scale) holds.
-        // The max value of abs(V) is 2^ValOrigBW - 1. Multiplying with a
-        // constant smaller than 2^(bitwidth(Val) - ValOrigBW) won't wrap.
-        int MaxScaleValueBW = Var.Val.getBitWidth() - ValOrigBW;
-        if (MaxScaleValueBW <= 0)
-          return false;
-        return Var.Scale.ule(
-            APInt::getMaxValue(MaxScaleValueBW).zext(Var.Scale.getBitWidth()));
-      };
       // Refine MinAbsVarIndex, if abs(Scale*V) >= abs(Scale) holds in the
       // presence of potentially wrapping math.
       if (MultiplyByScaleNoWrap(Var)) {
@@ -1345,7 +1346,8 @@ AliasResult BasicAAResult::aliasGEP(
                         SimplifyQuery(DL, DT, &AC, /*CxtI=*/Var0.CxtI
                                                        ? Var0.CxtI
                                                        : Var1.CxtI)))
-      MinAbsVarIndex = Var0.Scale.abs();
+      if (MultiplyByScaleNoWrap(Var0) && MultiplyByScaleNoWrap(Var1))
+        MinAbsVarIndex = Var0.Scale.abs();
   }
 
   if (MinAbsVarIndex) {
diff --git a/llvm/test/Analysis/BasicAA/gep-modulo.ll b/llvm/test/Analysis/BasicAA/gep-modulo.ll
index 159aaf94039d3..9ca69b2d7bbdd 100644
--- a/llvm/test/Analysis/BasicAA/gep-modulo.ll
+++ b/llvm/test/Analysis/BasicAA/gep-modulo.ll
@@ -393,3 +393,17 @@ entry:
 }
 
 declare void @llvm.assume(i1)
+
+define i64 @mul_may_overflow_var_nonzero_minabsvarindex_two_index(i64 %arg, ptr %arg1) {
+; CHECK-LABEL:  Function: mul_may_overflow_var_nonzero_minabsvarindex_two_index
+; CHECK-LABEL:  MayAlias:      i64* %getelementptr, i64* %getelementptr2
+bb:
+  %xor = xor i64 %arg, -9223372036854775808
+  %getelementptr = getelementptr i64, ptr %arg1, i64 %xor
+  %load = load i64, ptr %getelementptr, align 8
+  %getelementptr2 = getelementptr i64, ptr %arg1, i64 %arg
+  store i64 1, ptr %getelementptr2, align 8
+  %load3 = load i64, ptr %getelementptr, align 8
+  %mul = mul i64 %load, %load3
+  ret i64 %mul
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Approximate proof: https://alive2.llvm.org/ce/z/hTla9G This is not quite correct for the non-IsNegated case (src2), but I think we'd only hit that in cases where NSW is dropped earlier already (

Dest.Scale = -Dest.Scale;
Dest.IsNegated = false;
Dest.IsNSW = false;
).

@topperc topperc merged commit fff622f into llvm:main Apr 29, 2025
9 of 11 checks passed
@topperc topperc deleted the pr/basicaa branch April 29, 2025 21:10
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 29, 2025

LLVM Buildbot has detected a new failure on builder openmp-s390x-linux running on systemz-1 while building llvm at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/11076

Here is the relevant piece of the build log for the reference
Step 6 (test-openmp) failure: test (failure)
******************** TEST 'libomp :: tasking/issue-94260-2.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp   -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************


IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
… >= abs(Scale) (llvm#137755)

Similar to 1b7ef6a, add a check to only
set MinAbsVarIndex if abs(Scale*V0) and abs((-Scale)*V1) won't wrap. In
the absence of IsNSW, try to use the bitwidths of the original V and
Scale to rule out wrapping
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
… >= abs(Scale) (llvm#137755)

Similar to 1b7ef6a, add a check to only
set MinAbsVarIndex if abs(Scale*V0) and abs((-Scale)*V1) won't wrap. In
the absence of IsNSW, try to use the bitwidths of the original V and
Scale to rule out wrapping
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants