-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[BasicAA] Treat IntToPtr(Argument) similarly to Argument in relation to function-local objects. #134505
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
base: main
Are you sure you want to change the base?
Conversation
…to function-local objects. Given a IntToPtr or an argument, or of a extract from an argument that can come up from coerced call parameter, it cannot alias with an Object that is function local. My understanding is that essentially from dataflow alone we can prove noalias, not requiring any provenance or whether the pointer escapes via the inttoptr. That is true even for noalias arguments if my reading of D101541 and the Pointer Aliasing Rules is correct, but I am really not an expert. See https://reviews.llvm.org/D101541 for the last time this came up in the altered test cases.
@llvm/pr-subscribers-llvm-analysis Author: David Green (davemgreen) ChangesGiven a IntToPtr or an argument, or of an extract from an argument (that can come up from coerced call parameter), it cannot alias with an object that is function local. My understanding is that essentially from dataflow alone we can prove noalias, not requiring any provenance or whether the pointer escapes via the inttoptr. That is true even for noalias arguments if my reading of D101541 and the Pointer Aliasing Rules is correct, but I am really not an expert. See https://reviews.llvm.org/D101541 for the last time this came up in the altered test cases. Full diff: https://github.com/llvm/llvm-project/pull/134505.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 4d1a95a0c4b43..66b2c1050d59c 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1533,6 +1533,23 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
return Alias;
}
+// Return true for an Argument, IntToPtr(Argument) or
+// IntToPtr(ExtractValue(Argument)). These are all known
+// to not alias with FunctionLocal objects and can come up
+// from coerced function arguments.
+static bool isArgumentOrArgumentLike(const Value *V) {
+ if (isa<Argument>(V))
+ return true;
+ if (auto *P = dyn_cast<IntToPtrInst>(V)) {
+ const Value *POp = P->getOperand(0);
+ if (isa<Argument>(POp))
+ return true;
+ if (auto *E = dyn_cast<ExtractValueInst>(POp))
+ return isa<Argument>(E->getOperand(0));
+ }
+ return false;
+}
+
/// Provides a bunch of ad-hoc rules to disambiguate in common cases, such as
/// array references.
AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
@@ -1585,8 +1602,8 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
// Function arguments can't alias with things that are known to be
// unambigously identified at the function level.
- if ((isa<Argument>(O1) && isIdentifiedFunctionLocal(O2)) ||
- (isa<Argument>(O2) && isIdentifiedFunctionLocal(O1)))
+ if ((isArgumentOrArgumentLike(O1) && isIdentifiedFunctionLocal(O2)) ||
+ (isArgumentOrArgumentLike(O2) && isIdentifiedFunctionLocal(O1)))
return AliasResult::NoAlias;
// If one pointer is the result of a call/invoke or load and the other is a
diff --git a/llvm/test/Analysis/BasicAA/noalias-inttoptr.ll b/llvm/test/Analysis/BasicAA/noalias-inttoptr.ll
index 24bbcc55b3202..4e48dd048cf6b 100644
--- a/llvm/test/Analysis/BasicAA/noalias-inttoptr.ll
+++ b/llvm/test/Analysis/BasicAA/noalias-inttoptr.ll
@@ -24,10 +24,10 @@ define void @test2(i64 %Q_as_int) {
ret void
}
-; Verify that escaped noalias parameter may alias inttoptr
+; Verify that escaped noalias parameter are no alias inttoptr
define void @test3(ptr noalias %P, i64 %Q_as_int) {
; CHECK-LABEL: Function: test3:
- ; CHECK: MayAlias: i8* %P, i8* %Q
+ ; CHECK: NoAlias: i8* %P, i8* %Q
call void @escape(ptr %P)
%Q = inttoptr i64 %Q_as_int to ptr
store i8 0, ptr %P
@@ -35,10 +35,10 @@ define void @test3(ptr noalias %P, i64 %Q_as_int) {
ret void
}
-; Verify that escaped alloca may alias inttoptr
+; Verify that escaped alloca are noalias inttoptr
define void @test4(i64 %Q_as_int) {
; CHECK-LABEL: Function: test4:
- ; CHECK: MayAlias: i8* %P, i8* %Q
+ ; CHECK: NoAlias: i8* %P, i8* %Q
%P = alloca i8
call void @escape(ptr %P)
%Q = inttoptr i64 %Q_as_int to ptr
@@ -58,3 +58,15 @@ define void @test5(i64 %Q_as_int) {
store i8 1, ptr %Q
ret void
}
+
+; Verify that extractvalue of a coerced argument are noalias a function local object
+define void @test6([2 x i64] %Q.coerce) {
+ ; CHECK-LABEL: Function: test6:
+ ; CHECK: NoAlias: i8* %P, i8* %Q
+ %P = alloca i8
+ %Q_as_int = extractvalue [2 x i64] %Q.coerce, 1
+ %Q = inttoptr i64 %Q_as_int to ptr
+ store i8 0, ptr %P
+ store i8 1, ptr %Q
+ ret void
+}
|
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.
This is not correct for inttoptr. In test3, if %Q_as_int == ptrtoint(%P)
and %P
has escaped (and thus its provenance may be exposed), then inttoptr(%Q_as_int)
may alias %P
.
test6 is fine because the alloca does not escape -- but that case is already handled (https://llvm.godbolt.org/z/18zvW8Trz) based on escape source reasoning.
define void @test3(ptr noalias %P, i64 %Q_as_int) { | ||
; CHECK-LABEL: Function: test3: | ||
; CHECK: MayAlias: i8* %P, i8* %Q | ||
; CHECK: NoAlias: i8* %P, i8* %Q |
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.
this is not correct. %P and %Q can alias if the caller did ptr2int on %Q.
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.
ah, sorry, I missed the noalias bit.
I'm sympathetic to accepting this as valid, but I'm not sure LLVM is ready for that. Probably in most places we only check nolias against other pointer arguments and not against integers.
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.
I don't really see how we could accept this, regardless of noalias. Maybe to make it more explicit, we expect that the following code is not UB (at the LLVM IR level):
void test(int * restrict P, uintptr_t Q_as_int) {
if ((uintptr_t)P == Q_as_int) {
(int *)Q_as_int = 1; // Can alias P.
}
}
Here (uintptr_t)P
exposes the provenance of P
and then (int *)Q_as_int
chooses it.
(In the test case, the call to @escape
stands in for the provenance exposure.)
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.
That's one interpretation of noalias. But you can also choose to say that P cannot alias with the result of int2ptr since noalias means you cannot do any access through another pointer that points to the same location as P.
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.
So you are suggesting that for a noalias
pointer %P
, doing store inttoptr(ptrtoint(%P))
would be UB? I don't think that is viable due to frontend requirements.
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.
true, it's not easy to get working. the only solution for now is to lose precision and mark these as may alias.
OK. There is likely something wrong with my head-canon on aliasing and provenance, it is quite a complex subject that I understand hasn't necessarily been nailed down yet. I think the one I am interested in is aliasing between allocas that escape and coerced args, not noalias args (although the case I am looking at contains both). Is it just for noalias args that this would be incorrect, or allocas too? I think the thing I don't understand is how the parent call could have Or is it that inttoptr recreates provenance at the current time it is executed from address, and so (along with 33896) inttoptrs can have some level of side-effects and cannot be moved past an alloc call? i.e. we are aiming for a pvni-ae model even if we are not there yet. (I might be able to change how the arguments are coerced, side-stepping all of this. It wouldn't work in all cases, I need to check). |
You are right that callers cannot guess the address of allocas. So if you get an integer as argument and then do int2ptr that cannot alias with an alloca whose address hasn't escaped & has sufficient provenance (not passed to a function call & not an argument to ptr2int). Just doing icmp is not sufficient for not being able to deduce noalias (only observes address, doesn't give provenance). |
The situation for allocas is pretty similar. Adjusting the example from https://github.com/llvm/llvm-project/pull/134505/files/1c308aa8adf9ddb03726d2d35dfa1689a438f8d5#r2031886105 you get (using C only for brevity):
This function can either do nothing or print 1, but it can't print 0. The caller does not know the address of P, but it could still just guess luckily.
Yes, some kind of pnvi-ae style model is my current operating assumption.
Changing argument coercion would be ideal. No matter how you turn it, passing pointers though integers will always do bad things with provenance. |
…n to function-local objects. This is a much smaller, technically orthoganal patch similar to llvm#134505. It states that a extractvalue(Argument) can be treated like an Argument for alias analysis, where the extractelement acts like a phi / copy. No inttotr here. I'm hoping for this operations this is perfectly sensible, but it is quite a complex area.
I'll see if I can change the type of the coerced argument and hopefully that is more straight forward. |
…n to function-local objects. This is a much smaller, technically orthoganal patch similar to llvm#134505. It states that a extractvalue(Argument) can be treated like an Argument for alias analysis, where the extractelement acts like a phi / copy. No inttotr here. I'm hoping for this operations this is perfectly sensible, but it is quite a complex area.
Given a IntToPtr or an argument, or of an extract from an argument (that can come up from coerced call parameter), it cannot alias with an object that is function local. My understanding is that essentially from dataflow alone we can prove noalias, not requiring any provenance or whether the pointer escapes via the inttoptr. That is true even for noalias arguments if my reading of D101541 and the Pointer Aliasing Rules is correct, but I am really not an expert.
See https://reviews.llvm.org/D101541 for the last time this came up in the altered test cases.