Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BasicAA] Handle disjoint or as add in DecomposeGEP. #78209

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

davemgreen
Copy link
Collaborator

This removes the MaskedValueIsZero check in decomposing geps in BasicAA, using the isDisjoint flags instead. This relies on the disjoint flags being present. The other alternative is to keep the old MaskedValueIsZero check too.

This removes the MaskedValueIsZero check in decomposing geps in BasicAA, using
the isDisjoint flags instead. This relies on the disjoint flags being present.
The other alternative is to keep the old MaskedValueIsZero check too.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-llvm-analysis

Author: David Green (davemgreen)

Changes

This removes the MaskedValueIsZero check in decomposing geps in BasicAA, using the isDisjoint flags instead. This relies on the disjoint flags being present. The other alternative is to keep the old MaskedValueIsZero check too.


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/BasicAliasAnalysis.cpp (+2-4)
  • (modified) llvm/test/Analysis/BasicAA/gep-alias.ll (+16-1)
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index a4a0846df7af15..ab1ab106659e7e 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -393,10 +393,8 @@ static LinearExpression GetLinearExpression(
         // further.
         return Val;
       case Instruction::Or:
-        // X|C == X+C if all the bits in C are unset in X.  Otherwise we can't
-        // analyze it.
-        if (!MaskedValueIsZero(BOp->getOperand(0), RHSC->getValue(),
-                               SimplifyQuery(DL, DT, AC, BOp)))
+        // X|C == X+C if it is disjoint.  Otherwise we can't analyze it.
+        if (!cast<PossiblyDisjointInst>(BOp)->isDisjoint())
           return Val;
 
         [[fallthrough]];
diff --git a/llvm/test/Analysis/BasicAA/gep-alias.ll b/llvm/test/Analysis/BasicAA/gep-alias.ll
index 30298625426641..132cc496b8f7d9 100644
--- a/llvm/test/Analysis/BasicAA/gep-alias.ll
+++ b/llvm/test/Analysis/BasicAA/gep-alias.ll
@@ -114,7 +114,7 @@ define i32 @test5_as1_same_size(ptr addrspace(1) %p, i16 %i) {
 define i32 @test6(ptr %p, i64 %i1) {
   %i = shl i64 %i1, 2
   %pi = getelementptr i32, ptr %p, i64 %i
-  %i.next = or i64 %i, 1
+  %i.next = or disjoint i64 %i, 1
   %pi.next = getelementptr i32, ptr %p, i64 %i.next
   %x = load i32, ptr %pi
   store i32 42, ptr %pi.next
@@ -125,6 +125,21 @@ define i32 @test6(ptr %p, i64 %i1) {
 ; CHECK: ret i32 0
 }
 
+; P[i] != p[(i*4)|2048] with disjoint or
+define i32 @test6_higheror(ptr %p, i64 %i1) {
+  %i = shl nuw nsw i64 %i1, 2
+  %pi = getelementptr i32, ptr %p, i64 %i
+  %i.next = or disjoint i64 %i, 2048
+  %pi.next = getelementptr i32, ptr %p, i64 %i.next
+  %x = load i32, ptr %pi
+  store i32 42, ptr %pi.next
+  %y = load i32, ptr %pi
+  %z = sub i32 %x, %y
+  ret i32 %z
+; CHECK-LABEL: @test6_higheror(
+; CHECK: ret i32 0
+}
+
 ; P[1] != P[i*4]
 define i32 @test7(ptr %p, i64 %i) {
   %pi = getelementptr i32, ptr %p, i64 1

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

@davemgreen davemgreen merged commit d69efa4 into llvm:main Jan 16, 2024
4 of 5 checks passed
@davemgreen davemgreen deleted the gh-aa-disjointor branch January 16, 2024 09:22
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This removes the MaskedValueIsZero check in decomposing geps in BasicAA, using
the isDisjoint flags instead. This relies on the disjoint flags being present
when AA is ran. The alternative would be to keep the old MaskedValueIsZero check
too if this causes issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants