-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AA] Assert that alias() arguments are pointers #138242
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
Conversation
Assert instead of returning NoAlias for non-pointers. This makes sure that people don't confused alias (working on locations) with getModRefInfo (working on instructions).
@llvm/pr-subscribers-llvm-analysis Author: Nikita Popov (nikic) ChangesAssert instead of returning NoAlias for non-pointers. This makes sure that people don't confuse alias (working on locations) with getModRefInfo (working on instructions). Full diff: https://github.com/llvm/llvm-project/pull/138242.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp
index efabf69b06047..f4946c30de9bc 100644
--- a/llvm/lib/Analysis/AliasAnalysis.cpp
+++ b/llvm/lib/Analysis/AliasAnalysis.cpp
@@ -110,6 +110,9 @@ AliasResult AAResults::alias(const MemoryLocation &LocA,
AliasResult AAResults::alias(const MemoryLocation &LocA,
const MemoryLocation &LocB, AAQueryInfo &AAQI,
const Instruction *CtxI) {
+ assert(LocA.Ptr->getType()->isPointerTy() &&
+ LocB.Ptr->getType()->isPointerTy() &&
+ "Can only call alias() on pointers");
AliasResult Result = AliasResult::MayAlias;
if (EnableAATrace) {
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index a46edc0b75f54..2de9bb502baf4 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1574,9 +1574,6 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
if (isValueEqualInPotentialCycles(V1, V2, AAQI))
return AliasResult::MustAlias;
- if (!V1->getType()->isPointerTy() || !V2->getType()->isPointerTy())
- return AliasResult::NoAlias; // Scalars cannot alias each other
-
// Figure out what objects these things are pointing to if we can.
const Value *O1 = getUnderlyingObject(V1, MaxLookupSearchDepth);
const Value *O2 = getUnderlyingObject(V2, MaxLookupSearchDepth);
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
might be good to also update the doc comment, if it isn’t already
Assert instead of returning NoAlias for non-pointers. This makes sure that people don't confuse alias (working on locations) with getModRefInfo (working on instructions).
Assert instead of returning NoAlias for non-pointers. This makes sure that people don't confuse alias (working on locations) with getModRefInfo (working on instructions).
Assert instead of returning NoAlias for non-pointers. This makes sure that people don't confuse alias (working on locations) with getModRefInfo (working on instructions).
Assert instead of returning NoAlias for non-pointers. This makes sure that people don't confuse alias (working on locations) with getModRefInfo (working on instructions).
Hi, I'm debugging a problem after this commit with one of the Enzyme (https://github.com/EnzymeAD/) tests. There's some code that doesn't call
the call to
and running the test case under lldb,
should the check in |
@slackito Most likely the code is calling MemoryLocation::getForArgument on a non-pointer argument. |
Thanks for the advice! I'll fix it on my end then. |
Assert instead of returning NoAlias for non-pointers. This makes sure that people don't confuse alias (working on locations) with getModRefInfo (working on instructions).