[globals-aa] Improved isNonEscapingGlobalNoAlias. #127707
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A non-escaping global should never alias with non-pointer values and `ptr null`. This should improve disambiguation of global pointers with relation to Flang runtime calls (given that `nosync nocallback` attributes are properly set). It also seems to be an obvious improvement with little overhead.
llvmbot
added
the
llvm:analysis
|
@llvm/pr-subscribers-llvm-analysis Author: Slava Zakharin (vzakhari) ChangesA non-escaping global should never alias with non-pointer values Full diff: https://github.com/llvm/llvm-project/pull/127707.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/GlobalsModRef.h b/llvm/include/llvm/Analysis/GlobalsModRef.h
index ab8ab8295b556..36a95e095aaa5 100644
--- a/llvm/include/llvm/Analysis/GlobalsModRef.h
+++ b/llvm/include/llvm/Analysis/GlobalsModRef.h
@@ -118,7 +118,8 @@ class GlobalsAAResult : public AAResultBase {
bool AnalyzeIndirectGlobalMemory(GlobalVariable *GV);
void CollectSCCMembership(CallGraph &CG);
- bool isNonEscapingGlobalNoAlias(const GlobalValue *GV, const Value *V);
+ bool isNonEscapingGlobalNoAlias(const GlobalValue *GV, const Value *V,
+ const Instruction *CtxI);
ModRefInfo getModRefInfoForArgument(const CallBase *Call,
const GlobalValue *GV, AAQueryInfo &AAQI);
};
diff --git a/llvm/lib/Analysis/GlobalsModRef.cpp b/llvm/lib/Analysis/GlobalsModRef.cpp
index 1ceb1b2629418..ade9ffe333b2d 100644
--- a/llvm/lib/Analysis/GlobalsModRef.cpp
+++ b/llvm/lib/Analysis/GlobalsModRef.cpp
@@ -713,7 +713,8 @@ static bool isNonEscapingGlobalNoAliasWithLoad(const GlobalValue *GV,
// active, or to be forced to operate as a module pass that cannot co-exist
// with an alias analysis such as GMR.
bool GlobalsAAResult::isNonEscapingGlobalNoAlias(const GlobalValue *GV,
- const Value *V) {
+ const Value *V,
+ const Instruction *CtxI) {
// In order to know that the underlying object cannot alias the
// non-addr-taken global, we must know that it would have to be an escape.
// Thus if the underlying object is a function argument, a load from
@@ -762,6 +763,18 @@ bool GlobalsAAResult::isNonEscapingGlobalNoAlias(const GlobalValue *GV,
continue;
}
+ // A non-addr-taken global cannot alias with any non-pointer value.
+ if (!Input->getType()->isPointerTy())
+ continue;
+
+ if (CtxI) {
+ // Null pointer cannot alias with a non-addr-taken global.
+ const Function *F = CtxI->getFunction();
+ if (const ConstantPointerNull *CPN = dyn_cast<ConstantPointerNull>(Input))
+ if (!NullPointerIsDefined(F, CPN->getType()->getAddressSpace()))
+ continue;
+ }
+
// Recurse through a limited number of selects, loads and PHIs. This is an
// arbitrary depth of 4, lower numbers could be used to fix compile time
// issues if needed, but this is generally expected to be only be important
@@ -820,7 +833,7 @@ bool GlobalsAAResult::invalidate(Module &, const PreservedAnalyses &PA,
/// address of the global isn't taken.
AliasResult GlobalsAAResult::alias(const MemoryLocation &LocA,
const MemoryLocation &LocB,
- AAQueryInfo &AAQI, const Instruction *) {
+ AAQueryInfo &AAQI, const Instruction *CtxI) {
// Get the base object these pointers point to.
const Value *UV1 =
getUnderlyingObject(LocA.Ptr->stripPointerCastsForAliasAnalysis());
@@ -856,7 +869,7 @@ AliasResult GlobalsAAResult::alias(const MemoryLocation &LocA,
if ((GV1 || GV2) && GV1 != GV2) {
const GlobalValue *GV = GV1 ? GV1 : GV2;
const Value *UV = GV1 ? UV2 : UV1;
- if (isNonEscapingGlobalNoAlias(GV, UV))
+ if (isNonEscapingGlobalNoAlias(GV, UV, CtxI))
return AliasResult::NoAlias;
}
@@ -920,7 +933,7 @@ ModRefInfo GlobalsAAResult::getModRefInfoForArgument(const CallBase *Call,
!all_of(Objects, [&](const Value *V) {
return this->alias(MemoryLocation::getBeforeOrAfter(V),
MemoryLocation::getBeforeOrAfter(GV), AAQI,
- nullptr) == AliasResult::NoAlias;
+ Call) == AliasResult::NoAlias;
}))
return ConservativeResult;
diff --git a/llvm/test/Analysis/GlobalsModRef/nonescaping-noalias.ll b/llvm/test/Analysis/GlobalsModRef/nonescaping-noalias.ll
index 4a2c10ca55cdc..eed93cf0df8ef 100644
--- a/llvm/test/Analysis/GlobalsModRef/nonescaping-noalias.ll
+++ b/llvm/test/Analysis/GlobalsModRef/nonescaping-noalias.ll
@@ -175,3 +175,24 @@ entry:
%v = load i32, ptr @g1
ret i32 %v
}
+
+define i32 @test6(ptr %param) {
+; Ensure that we can fold a store to a load of a global across a set of
+; calls that cannot use in any way a non-escaping global.
+;
+; CHECK-LABEL: @test6(
+; CHECK: store i32 42, ptr @g1
+; CHECK-NOT: load i32
+; CHECK: ret i32 42
+entry:
+ store i32 42, ptr @g1
+ %1 = call ptr @_FortranAioBeginExternalFormattedOutput(ptr null, i64 3, ptr null, i32 6, ptr null, i32 2)
+ %2 = call i1 @_FortranAioOutputAscii(ptr %1, ptr null, i64 4)
+ %3 = call i32 @_FortranAioEndIoStatement(ptr %1)
+ %v = load i32, ptr @g1
+ ret i32 %v
+}
+declare ptr @_FortranAioBeginExternalFormattedOutput(ptr, i64, ptr, i32, ptr, i32) #0
+declare zeroext i1 @_FortranAioOutputAscii(ptr, ptr, i64) #0
+declare i32 @_FortranAioEndIoStatement(ptr) #0
+attributes #0 = { nocallback nosync }
|
llvm/lib/Analysis/GlobalsModRef.cpp
Outdated
| @@ -762,6 +763,18 @@ bool GlobalsAAResult::isNonEscapingGlobalNoAlias(const GlobalValue *GV, | |||
| continue; | |||
| } | |||
|
|
|||
| // A non-addr-taken global cannot alias with any non-pointer value. | |||
| if (!Input->getType()->isPointerTy()) | |||
There was a problem hiding this comment.
How do we end up with a value that isn't a pointer here? alias() itself should only be queried with pointers (it doesn't make sense to ask whether a non-pointer aliases anything). And the recursive steps in isNonEscapingGlobalNoAlias should produce pointers if the input is a pointer.
There was a problem hiding this comment.
We end up here from line 934.
I guess a pointer captured via ptrtoint or any other indirect "conversion" to a non-pointer value might be considered aliasing with this non-pointer.
There was a problem hiding this comment.
I think that's a bug in that code; we're only supposed to be trying to analyze pointers.
A ptrtoint would be considered an escape, so we don't need to try to analyze integers to figure out if they were originally pointers.
There was a problem hiding this comment.
Note that BasicAAResult::aliasCheck seems to expect that any of the values can have non-pointer type:
So I am not sure there is a strict requirement that alias can only be called with pointers. I can easily move this check to the code around line 934, but it seems it would be more appropriate staying here.
There was a problem hiding this comment.
I have no idea why we don't forbid alias queries on non-pointers. CC @nikic.
In any case, you can hoist this check out of the loop.
llvm/lib/Analysis/GlobalsModRef.cpp
Outdated
|
|
||
| if (CtxI) { | ||
| // Null pointer cannot alias with a non-addr-taken global. | ||
| const Function *F = CtxI->getFunction(); |
There was a problem hiding this comment.
Sink the variable declaration into the if?
llvm/lib/Analysis/GlobalsModRef.cpp
Outdated
| continue; | ||
|
|
||
| if (CtxI) | ||
| if (const ConstantPointerNull *CPN = |
There was a problem hiding this comment.
You can use "auto" here.
llvm/lib/Analysis/GlobalsModRef.cpp
Outdated
| @@ -762,6 +763,18 @@ bool GlobalsAAResult::isNonEscapingGlobalNoAlias(const GlobalValue *GV, | |||
| continue; | |||
| } | |||
|
|
|||
| // A non-addr-taken global cannot alias with any non-pointer value. | |||
| if (!Input->getType()->isPointerTy()) | |||
There was a problem hiding this comment.
I have no idea why we don't forbid alias queries on non-pointers. CC @nikic.
In any case, you can hoist this check out of the loop.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/13308 Here is the relevant piece of the build log for the reference |
vzakhari
added a commit
to vzakhari/llvm-project
that referenced
this pull request
Feb 21, 2025
This change is inspired by a case in facerec benchmark, where performance of scalar code may improve by about 6%@aarch64 due to getting rid of redundant loads from Fortran descriptors. These descriptors are corresponding to subroutine local ALLOCATABLE, SAVE variables. The scalar loop nest in LocalMove subroutine contains call to Fortran runtime IO functions, and LLVM globals-aa analysis cannot prove that these calls do not modify the globalized descriptors with internal linkage. This patch sets and propagates llvm.memory_effects attribute for fir.call operations calling Fortran runtime functions. In particular, it tries to set the Other memory effect to NoModRef. The Other memory effect includes accesses to globals and captured pointers, so we cannot set it for functions taking Fortran descriptors with one exception for calls where the Fortran descriptor arguments are all null. As long as different calls to the same Fortran runtime function may have different attributes, I decided to attach the attributes to the calls rather than functions. Moreover, attaching the attributes to func.func will require propagating these attributes to llvm.func, which is not happening right now. In addition to llvm.memory_effects, the new pass sets llvm.nosync and llvm.nocallback attributes that may also help LLVM alias analysis (e.g. see llvm#127707). These attributes are ignored currently. I will support them in LLVM IR dialect in a separate patch. I also added another pass for developers to be able to print declarations/calls of all Fortran runtime functions that are recognized by the attributes setting pass. It should help with maintenance of the LIT tests.
vzakhari
added a commit
that referenced
this pull request
Feb 24, 2025
#128093) This change is inspired by a case in facerec benchmark, where performance of scalar code may improve by about 6%@aarch64 due to getting rid of redundant loads from Fortran descriptors. These descriptors are corresponding to subroutine local ALLOCATABLE, SAVE variables. The scalar loop nest in LocalMove subroutine contains call to Fortran runtime IO functions, and LLVM globals-aa analysis cannot prove that these calls do not modify the globalized descriptors with internal linkage. This patch sets and propagates llvm.memory_effects attribute for fir.call operations calling Fortran runtime functions. In particular, it tries to set the Other memory effect to NoModRef. The Other memory effect includes accesses to globals and captured pointers, so we cannot set it for functions taking Fortran descriptors with one exception for calls where the Fortran descriptor arguments are all null. As long as different calls to the same Fortran runtime function may have different attributes, I decided to attach the attributes to the calls rather than functions. Moreover, attaching the attributes to func.func will require propagating these attributes to llvm.func, which is not happening right now. In addition to llvm.memory_effects, the new pass sets llvm.nosync and llvm.nocallback attributes that may also help LLVM alias analysis (e.g. see #127707). These attributes are ignored currently. I will support them in LLVM IR dialect in a separate patch. I also added another pass for developers to be able to print declarations/calls of all Fortran runtime functions that are recognized by the attributes setting pass. It should help with maintenance of the LIT tests.
vzakhari
added a commit
that referenced
this pull request
Feb 24, 2025
… runtime. (#128093)" This change is inspired by a case in facerec benchmark, where performance of scalar code may improve by about 6%@aarch64 due to getting rid of redundant loads from Fortran descriptors. These descriptors are corresponding to subroutine local ALLOCATABLE, SAVE variables. The scalar loop nest in LocalMove subroutine contains call to Fortran runtime IO functions, and LLVM globals-aa analysis cannot prove that these calls do not modify the globalized descriptors with internal linkage. This patch sets and propagates llvm.memory_effects attribute for fir.call operations calling Fortran runtime functions. In particular, it tries to set the Other memory effect to NoModRef. The Other memory effect includes accesses to globals and captured pointers, so we cannot set it for functions taking Fortran descriptors with one exception for calls where the Fortran descriptor arguments are all null. As long as different calls to the same Fortran runtime function may have different attributes, I decided to attach the attributes to the calls rather than functions. Moreover, attaching the attributes to func.func will require propagating these attributes to llvm.func, which is not happening right now. In addition to llvm.memory_effects, the new pass sets llvm.nosync and llvm.nocallback attributes that may also help LLVM alias analysis (e.g. see #127707). These attributes are ignored currently. I will support them in LLVM IR dialect in a separate patch. I also added another pass for developers to be able to print declarations/calls of all Fortran runtime functions that are recognized by the attributes setting pass. It should help with maintenance of the LIT tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A non-escaping global should never alias with non-pointer values
and
ptr null. This should improve disambiguation of globalpointers with relation to Flang runtime calls (given that
nosync nocallbackattributes are properly set).It also seems to be an obvious improvement with little overhead.