[llvm] [ArgPromotion] Perform alias analysis on actual arguments of Calls (PR #106216)
Hari Limaye via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 24 15:24:27 PDT 2024
https://github.com/hazzlim updated https://github.com/llvm/llvm-project/pull/106216
>From d10cb42c7c7f55d06ed093c334cd82adad524730 Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Thu, 22 Aug 2024 21:52:44 +0000
Subject: [PATCH 1/6] [ArgPromotion] Perform alias analysis on actual arguments
of Calls
Teach Argument Promotion to perform alias analysis on actual arguments
of Calls to a Function, to try to prove that all Calls to the Function
do not modify the memory pointed to by an argument. This surfaces more
opportunities to perform Argument Promotion in cases where simply
looking at a Function's instructions is insufficient to prove that the
pointer argument is not invalidated before all loads from it.
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 58 +++++++++++++++----
.../ArgumentPromotion/actual-arguments.ll | 25 ++++----
2 files changed, 60 insertions(+), 23 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 1f9b546ed29996..428f809d3d45fb 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -489,7 +489,8 @@ static bool allCallersPassValidPointerForArgument(
/// parts it can be promoted into.
static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
unsigned MaxElements, bool IsRecursive,
- SmallVectorImpl<OffsetAndArgPart> &ArgPartsVec) {
+ SmallVectorImpl<OffsetAndArgPart> &ArgPartsVec,
+ bool ArgNotModified) {
// Quick exit for unused arguments
if (Arg->use_empty())
return true;
@@ -711,8 +712,10 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// If store instructions are allowed, the path from the entry of the function
// to each load may be not free of instructions that potentially invalidate
- // the load, and this is an admissible situation.
- if (AreStoresAllowed)
+ // the load, and this is an admissible situation. If we have already
+ // determined that the pointer Arg is not modified in the function (for all
+ // Calls) then we can similarly conclude analysis here.
+ if (AreStoresAllowed || ArgNotModified)
return true;
// Okay, now we know that the argument is only used by load instructions, and
@@ -760,6 +763,33 @@ static bool areTypesABICompatible(ArrayRef<Type *> Types, const Function &F,
});
}
+// Try to prove that all Calls to F do not modify the memory pointed to by Arg.
+// This can provide us with more opportunities to perform Argument Promotion in
+// cases where simply looking at a Function's instructions is insufficient to
+// prove that the pointer argument is not invalidated before all loads from it.
+static bool callDoesNotModifyArg(Function *F, unsigned ArgNo,
+ FunctionAnalysisManager &FAM) {
+ // Find all Users of F that are Calls, and see if they may modify Arg.
+ for (User *U : F->users()) {
+ auto *Call = dyn_cast<CallInst>(U);
+ if (!Call)
+ continue;
+
+ Value *ArgOp = Call->getArgOperand(ArgNo);
+ assert(ArgOp->getType()->isPointerTy() && "Argument must be Pointer Type!");
+
+ MemoryLocation Loc = MemoryLocation::getForArgument(Call, ArgNo, nullptr);
+
+ AAResults &AAR = FAM.getResult<AAManager>(*Call->getFunction());
+ // Bail out as soon as we find a Call where Arg may be modified.
+ if (isModSet(AAR.getModRefInfo(Call, Loc)))
+ return false;
+ }
+
+ // All Calls do not modify the Arg.
+ return true;
+}
+
/// PromoteArguments - This method checks the specified function to see if there
/// are any promotable arguments and if it is safe to promote the function (for
/// example, all callers are direct). If safe to promote some arguments, it
@@ -790,11 +820,13 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
return nullptr;
// First check: see if there are any pointer arguments! If not, quick exit.
- SmallVector<Argument *, 16> PointerArgs;
- for (Argument &I : F->args())
- if (I.getType()->isPointerTy())
- PointerArgs.push_back(&I);
- if (PointerArgs.empty())
+ SmallVector<unsigned, 16> PointerArgNos;
+ for (unsigned I = 0; I < F->arg_size(); ++I) {
+ Argument *Arg = F->getArg(I);
+ if (Arg->getType()->isPointerTy())
+ PointerArgNos.push_back(I);
+ }
+ if (PointerArgNos.empty())
return nullptr;
// Second check: make sure that all callers are direct callers. We can't
@@ -829,7 +861,8 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
// add it to ArgsToPromote.
DenseMap<Argument *, SmallVector<OffsetAndArgPart, 4>> ArgsToPromote;
unsigned NumArgsAfterPromote = F->getFunctionType()->getNumParams();
- for (Argument *PtrArg : PointerArgs) {
+ for (const auto &ArgIdx : PointerArgNos) {
+ Argument *PtrArg = F->getArg(ArgIdx);
// Replace sret attribute with noalias. This reduces register pressure by
// avoiding a register copy.
if (PtrArg->hasStructRetAttr()) {
@@ -843,10 +876,15 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
}
}
+ // Check if we can determine ahead of time that the argument is never
+ // modified by a call to this function.
+ bool ArgNotModified = callDoesNotModifyArg(F, ArgIdx, FAM);
+
// If we can promote the pointer to its value.
SmallVector<OffsetAndArgPart, 4> ArgParts;
- if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts)) {
+ if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts,
+ ArgNotModified)) {
SmallVector<Type *, 4> Types;
for (const auto &Pair : ArgParts)
Types.push_back(Pair.second.Ty);
diff --git a/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll b/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll
index 63366ba998c7bb..d52bfc3111779d 100644
--- a/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll
+++ b/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll
@@ -75,11 +75,9 @@ define internal i32 @test_cannot_promote_3(ptr %p, ptr nocapture readonly %test_
;
define internal i32 @test_can_promote_1(ptr %p, ptr nocapture readonly %test_c) {
; CHECK-LABEL: define {{[^@]+}}@test_can_promote_1
-; CHECK-SAME: (ptr [[P:%.*]], ptr nocapture readonly [[TEST_C:%.*]]) {
-; CHECK-NEXT: [[TEST_C_VAL:%.*]] = load i32, ptr [[TEST_C]], align 4
-; CHECK-NEXT: [[RES:%.*]] = call i32 @callee(ptr [[P]], i32 [[TEST_C_VAL]])
-; CHECK-NEXT: [[LTEST_C:%.*]] = load i32, ptr [[TEST_C]], align 4
-; CHECK-NEXT: [[SUM:%.*]] = add i32 [[LTEST_C]], [[RES]]
+; CHECK-SAME: (ptr [[P:%.*]], i32 [[TEST_C_0_VAL:%.*]]) {
+; CHECK-NEXT: [[RES:%.*]] = call i32 @callee(ptr [[P]], i32 [[TEST_C_0_VAL]])
+; CHECK-NEXT: [[SUM:%.*]] = add i32 [[TEST_C_0_VAL]], [[RES]]
; CHECK-NEXT: ret i32 [[SUM]]
;
%res = call i32 @callee(ptr %p, ptr %test_c)
@@ -99,11 +97,9 @@ define internal i32 @test_can_promote_1(ptr %p, ptr nocapture readonly %test_c)
;
define internal i32 @test_can_promote_2(ptr %p, ptr nocapture readonly %test_c) {
; CHECK-LABEL: define {{[^@]+}}@test_can_promote_2
-; CHECK-SAME: (ptr [[P:%.*]], ptr nocapture readonly [[TEST_C:%.*]]) {
-; CHECK-NEXT: [[TEST_C_VAL:%.*]] = load i32, ptr [[TEST_C]], align 4
-; CHECK-NEXT: [[RES:%.*]] = call i32 @callee(ptr [[P]], i32 [[TEST_C_VAL]])
-; CHECK-NEXT: [[LTEST_C:%.*]] = load i32, ptr [[TEST_C]], align 4
-; CHECK-NEXT: [[SUM:%.*]] = add i32 [[LTEST_C]], [[RES]]
+; CHECK-SAME: (ptr [[P:%.*]], i32 [[TEST_C_0_VAL:%.*]]) {
+; CHECK-NEXT: [[RES:%.*]] = call i32 @callee(ptr [[P]], i32 [[TEST_C_0_VAL]])
+; CHECK-NEXT: [[SUM:%.*]] = add i32 [[TEST_C_0_VAL]], [[RES]]
; CHECK-NEXT: ret i32 [[SUM]]
;
%res = call i32 @callee(ptr %p, ptr %test_c)
@@ -186,8 +182,10 @@ define i32 @caller_safe_args_1(i64 %n) {
; CHECK-NEXT: [[CALLER_C:%.*]] = alloca i32, align 4
; CHECK-NEXT: store i32 5, ptr [[CALLER_C]], align 4
; CHECK-NEXT: [[RES1:%.*]] = call i32 @test_cannot_promote_3(ptr [[P]], ptr [[CALLER_C]])
-; CHECK-NEXT: [[RES2:%.*]] = call i32 @test_can_promote_1(ptr [[P]], ptr [[CALLER_C]])
-; CHECK-NEXT: [[RES3:%.*]] = call i32 @test_can_promote_2(ptr [[P]], ptr [[CALLER_C]])
+; CHECK-NEXT: [[CALLER_C_VAL:%.*]] = load i32, ptr [[CALLER_C]], align 4
+; CHECK-NEXT: [[RES2:%.*]] = call i32 @test_can_promote_1(ptr [[P]], i32 [[CALLER_C_VAL]])
+; CHECK-NEXT: [[CALLER_C_VAL1:%.*]] = load i32, ptr [[CALLER_C]], align 4
+; CHECK-NEXT: [[RES3:%.*]] = call i32 @test_can_promote_2(ptr [[P]], i32 [[CALLER_C_VAL1]])
; CHECK-NEXT: [[RES12:%.*]] = add i32 [[RES1]], [[RES2]]
; CHECK-NEXT: [[RES:%.*]] = add i32 [[RES12]], [[RES3]]
; CHECK-NEXT: ret i32 [[RES]]
@@ -215,7 +213,8 @@ define i32 @caller_safe_args_2(i64 %n, ptr %p) {
; CHECK-NEXT: call void @memset(ptr [[P]], i64 0, i64 [[N]])
; CHECK-NEXT: [[CALLER_C:%.*]] = alloca i32, align 4
; CHECK-NEXT: store i32 5, ptr [[CALLER_C]], align 4
-; CHECK-NEXT: [[RES:%.*]] = call i32 @test_can_promote_2(ptr [[P]], ptr [[CALLER_C]])
+; CHECK-NEXT: [[CALLER_C_VAL:%.*]] = load i32, ptr [[CALLER_C]], align 4
+; CHECK-NEXT: [[RES:%.*]] = call i32 @test_can_promote_2(ptr [[P]], i32 [[CALLER_C_VAL]])
; CHECK-NEXT: ret i32 [[RES]]
;
call void @memset(ptr %p, i64 0, i64 %n)
>From 9aa4e64cd3faa460ffac5bbf62d4181fc5e0f7b0 Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Tue, 3 Sep 2024 14:42:02 +0000
Subject: [PATCH 2/6] Address review comments
- Bail on unexpected (non CallInst) use of function
- Move checks into findArgParts to improve clarity and only run when
necessary
- Remove FIXMEs from test
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 95 +++++++++----------
.../ArgumentPromotion/actual-arguments.ll | 4 -
2 files changed, 46 insertions(+), 53 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 428f809d3d45fb..05b1bf9f3cee1b 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -485,12 +485,39 @@ static bool allCallersPassValidPointerForArgument(
});
}
+// Try to prove that all Calls to F do not modify the memory pointed to by Arg,
+// using alias analysis local to each caller of F.
+static bool isArgUnmodifiedByAllCalls(Function *F, unsigned ArgNo,
+ FunctionAnalysisManager &FAM) {
+ // Check if all Users of F are Calls which do not modify Arg.
+ for (User *U : F->users()) {
+
+ // Bail if we find an unexpected (non CallInst) use of the function.
+ auto *Call = dyn_cast<CallInst>(U);
+ if (!Call)
+ return false;
+
+ Value *ArgOp = Call->getArgOperand(ArgNo);
+ assert(ArgOp->getType()->isPointerTy() && "Argument must be Pointer Type!");
+
+ MemoryLocation Loc = MemoryLocation::getForArgument(Call, ArgNo, nullptr);
+
+ AAResults &AAR = FAM.getResult<AAManager>(*Call->getFunction());
+ // Bail as soon as we find a Call where Arg may be modified.
+ if (isModSet(AAR.getModRefInfo(Call, Loc)))
+ return false;
+ }
+
+ // All Users are Calls which do not modify the Arg.
+ return true;
+}
+
/// Determine that this argument is safe to promote, and find the argument
/// parts it can be promoted into.
static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
unsigned MaxElements, bool IsRecursive,
SmallVectorImpl<OffsetAndArgPart> &ArgPartsVec,
- bool ArgNotModified) {
+ FunctionAnalysisManager &FAM) {
// Quick exit for unused arguments
if (Arg->use_empty())
return true;
@@ -712,17 +739,21 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// If store instructions are allowed, the path from the entry of the function
// to each load may be not free of instructions that potentially invalidate
- // the load, and this is an admissible situation. If we have already
- // determined that the pointer Arg is not modified in the function (for all
- // Calls) then we can similarly conclude analysis here.
- if (AreStoresAllowed || ArgNotModified)
+ // the load, and this is an admissible situation.
+ if (AreStoresAllowed)
return true;
// Okay, now we know that the argument is only used by load instructions, and
- // it is safe to unconditionally perform all of them. Use alias analysis to
- // check to see if the pointer is guaranteed to not be modified from entry of
- // the function to each of the load instructions.
+ // it is safe to unconditionally perform all of them.
+
+ // If we can determine that no call to the Function modifies the memory
+ // pointed to by Arg, through alias analysis using actual arguments in the
+ // callers, we know that it is guaranteed to be safe to promote the argument.
+ if (isArgUnmodifiedByAllCalls(Arg->getParent(), Arg->getArgNo(), FAM))
+ return true;
+ // Otherwise, use alias analysis to check if the pointer is guaranteed to not
+ // be modified from entry of the function to each of the load instructions.
for (LoadInst *Load : Loads) {
// Check to see if the load is invalidated from the start of the block to
// the load itself.
@@ -763,33 +794,6 @@ static bool areTypesABICompatible(ArrayRef<Type *> Types, const Function &F,
});
}
-// Try to prove that all Calls to F do not modify the memory pointed to by Arg.
-// This can provide us with more opportunities to perform Argument Promotion in
-// cases where simply looking at a Function's instructions is insufficient to
-// prove that the pointer argument is not invalidated before all loads from it.
-static bool callDoesNotModifyArg(Function *F, unsigned ArgNo,
- FunctionAnalysisManager &FAM) {
- // Find all Users of F that are Calls, and see if they may modify Arg.
- for (User *U : F->users()) {
- auto *Call = dyn_cast<CallInst>(U);
- if (!Call)
- continue;
-
- Value *ArgOp = Call->getArgOperand(ArgNo);
- assert(ArgOp->getType()->isPointerTy() && "Argument must be Pointer Type!");
-
- MemoryLocation Loc = MemoryLocation::getForArgument(Call, ArgNo, nullptr);
-
- AAResults &AAR = FAM.getResult<AAManager>(*Call->getFunction());
- // Bail out as soon as we find a Call where Arg may be modified.
- if (isModSet(AAR.getModRefInfo(Call, Loc)))
- return false;
- }
-
- // All Calls do not modify the Arg.
- return true;
-}
-
/// PromoteArguments - This method checks the specified function to see if there
/// are any promotable arguments and if it is safe to promote the function (for
/// example, all callers are direct). If safe to promote some arguments, it
@@ -820,13 +824,11 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
return nullptr;
// First check: see if there are any pointer arguments! If not, quick exit.
- SmallVector<unsigned, 16> PointerArgNos;
- for (unsigned I = 0; I < F->arg_size(); ++I) {
- Argument *Arg = F->getArg(I);
- if (Arg->getType()->isPointerTy())
- PointerArgNos.push_back(I);
- }
- if (PointerArgNos.empty())
+ SmallVector<Argument *, 16> PointerArgs;
+ for (Argument &I : F->args())
+ if (I.getType()->isPointerTy())
+ PointerArgs.push_back(&I);
+ if (PointerArgs.empty())
return nullptr;
// Second check: make sure that all callers are direct callers. We can't
@@ -861,8 +863,7 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
// add it to ArgsToPromote.
DenseMap<Argument *, SmallVector<OffsetAndArgPart, 4>> ArgsToPromote;
unsigned NumArgsAfterPromote = F->getFunctionType()->getNumParams();
- for (const auto &ArgIdx : PointerArgNos) {
- Argument *PtrArg = F->getArg(ArgIdx);
+ for (Argument *PtrArg : PointerArgs) {
// Replace sret attribute with noalias. This reduces register pressure by
// avoiding a register copy.
if (PtrArg->hasStructRetAttr()) {
@@ -876,15 +877,11 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM,
}
}
- // Check if we can determine ahead of time that the argument is never
- // modified by a call to this function.
- bool ArgNotModified = callDoesNotModifyArg(F, ArgIdx, FAM);
-
// If we can promote the pointer to its value.
SmallVector<OffsetAndArgPart, 4> ArgParts;
if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts,
- ArgNotModified)) {
+ FAM)) {
SmallVector<Type *, 4> Types;
for (const auto &Pair : ArgParts)
Types.push_back(Pair.second.Ty);
diff --git a/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll b/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll
index d52bfc3111779d..ca757a165fa4be 100644
--- a/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll
+++ b/llvm/test/Transforms/ArgumentPromotion/actual-arguments.ll
@@ -68,8 +68,6 @@ define internal i32 @test_cannot_promote_3(ptr %p, ptr nocapture readonly %test_
ret i32 %sum
}
-; FIXME: We should perform ArgPromotion here!
-;
; This is called only by @caller_safe_args_1, from which we can prove that
; %test_c does not alias %p for any Call to the function, so we can promote it.
;
@@ -89,8 +87,6 @@ define internal i32 @test_can_promote_1(ptr %p, ptr nocapture readonly %test_c)
ret i32 %sum
}
-; FIXME: We should perform ArgPromotion here!
-;
; This is called by multiple callers (@caller_safe_args_1, @caller_safe_args_2),
; from which we can prove that %test_c does not alias %p for any Call to the
; function, so we can promote it.
>From aa809e9dc8a9bf5b5fc0cdc6f56549bd7806879d Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Fri, 6 Sep 2024 14:37:02 +0000
Subject: [PATCH 3/6] Address review comments 2
- Remove redundant comments
- Refactor function signature
- Remove unnecessary assert
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 05b1bf9f3cee1b..8f3a603f914dd1 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -487,20 +487,17 @@ static bool allCallersPassValidPointerForArgument(
// Try to prove that all Calls to F do not modify the memory pointed to by Arg,
// using alias analysis local to each caller of F.
-static bool isArgUnmodifiedByAllCalls(Function *F, unsigned ArgNo,
+static bool isArgUnmodifiedByAllCalls(Argument *Arg,
FunctionAnalysisManager &FAM) {
- // Check if all Users of F are Calls which do not modify Arg.
- for (User *U : F->users()) {
+ for (User *U : Arg->getParent()->users()) {
// Bail if we find an unexpected (non CallInst) use of the function.
auto *Call = dyn_cast<CallInst>(U);
if (!Call)
return false;
- Value *ArgOp = Call->getArgOperand(ArgNo);
- assert(ArgOp->getType()->isPointerTy() && "Argument must be Pointer Type!");
-
- MemoryLocation Loc = MemoryLocation::getForArgument(Call, ArgNo, nullptr);
+ MemoryLocation Loc =
+ MemoryLocation::getForArgument(Call, Arg->getArgNo(), nullptr);
AAResults &AAR = FAM.getResult<AAManager>(*Call->getFunction());
// Bail as soon as we find a Call where Arg may be modified.
@@ -749,7 +746,7 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// If we can determine that no call to the Function modifies the memory
// pointed to by Arg, through alias analysis using actual arguments in the
// callers, we know that it is guaranteed to be safe to promote the argument.
- if (isArgUnmodifiedByAllCalls(Arg->getParent(), Arg->getArgNo(), FAM))
+ if (isArgUnmodifiedByAllCalls(Arg, FAM))
return true;
// Otherwise, use alias analysis to check if the pointer is guaranteed to not
>From 31621ccf4ce380b50115dc8fbbd9a1bf63c18493 Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Fri, 6 Sep 2024 17:28:53 +0000
Subject: [PATCH 4/6] Address review 3
- Compute the size of the memory region using the loads in the callee
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 8f3a603f914dd1..aaa9792c00249d 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -487,7 +487,7 @@ static bool allCallersPassValidPointerForArgument(
// Try to prove that all Calls to F do not modify the memory pointed to by Arg,
// using alias analysis local to each caller of F.
-static bool isArgUnmodifiedByAllCalls(Argument *Arg,
+static bool isArgUnmodifiedByAllCalls(Argument *Arg, const LocationSize &Size,
FunctionAnalysisManager &FAM) {
for (User *U : Arg->getParent()->users()) {
@@ -499,6 +499,9 @@ static bool isArgUnmodifiedByAllCalls(Argument *Arg,
MemoryLocation Loc =
MemoryLocation::getForArgument(Call, Arg->getArgNo(), nullptr);
+ // Refine the MemoryLocation Size using information from Loads of Arg.
+ Loc = Loc.getWithNewSize(Size);
+
AAResults &AAR = FAM.getResult<AAManager>(*Call->getFunction());
// Bail as soon as we find a Call where Arg may be modified.
if (isModSet(AAR.getModRefInfo(Call, Loc)))
@@ -743,10 +746,16 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// Okay, now we know that the argument is only used by load instructions, and
// it is safe to unconditionally perform all of them.
- // If we can determine that no call to the Function modifies the memory
- // pointed to by Arg, through alias analysis using actual arguments in the
+ // If we can determine that no call to the Function modifies the memory region
+ // accessed through Arg, through alias analysis using actual arguments in the
// callers, we know that it is guaranteed to be safe to promote the argument.
- if (isArgUnmodifiedByAllCalls(Arg, FAM))
+
+ // Compute the size of the memory region accessed by the Loads through Arg.
+ LocationSize Size = LocationSize::precise(0);
+ for (LoadInst *Load : Loads) {
+ Size = Size.unionWith(MemoryLocation::get(Load).Size);
+ }
+ if (isArgUnmodifiedByAllCalls(Arg, Size, FAM))
return true;
// Otherwise, use alias analysis to check if the pointer is guaranteed to not
>From 1e43fc2a9e2486bdf954d2f1caced02f51e0c0d4 Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Fri, 20 Sep 2024 16:08:15 +0000
Subject: [PATCH 5/6] Address review 4
- Fix incorrect calculation of the size of the memory region accessed by
the loads through Arg.
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index aaa9792c00249d..a7e88f0479721e 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -728,7 +728,8 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
append_range(ArgPartsVec, ArgParts);
sort(ArgPartsVec, llvm::less_first());
- // Make sure the parts are non-overlapping.
+ // Make sure the parts are non-overlapping. This also computes the size of the
+ // memory region accessed through Arg.
int64_t Offset = ArgPartsVec[0].first;
for (const auto &Pair : ArgPartsVec) {
if (Pair.first < Offset)
@@ -749,13 +750,7 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// If we can determine that no call to the Function modifies the memory region
// accessed through Arg, through alias analysis using actual arguments in the
// callers, we know that it is guaranteed to be safe to promote the argument.
-
- // Compute the size of the memory region accessed by the Loads through Arg.
- LocationSize Size = LocationSize::precise(0);
- for (LoadInst *Load : Loads) {
- Size = Size.unionWith(MemoryLocation::get(Load).Size);
- }
- if (isArgUnmodifiedByAllCalls(Arg, Size, FAM))
+ if (isArgUnmodifiedByAllCalls(Arg, LocationSize::precise(Offset), FAM))
return true;
// Otherwise, use alias analysis to check if the pointer is guaranteed to not
>From 3e343766289a2a2e1f830e8490f22360b0cac86b Mon Sep 17 00:00:00 2001
From: Hari Limaye <hari.limaye at arm.com>
Date: Tue, 24 Sep 2024 21:30:35 +0000
Subject: [PATCH 6/6] Remove Size calculation
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index a7e88f0479721e..c5fd36db47923e 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -487,7 +487,7 @@ static bool allCallersPassValidPointerForArgument(
// Try to prove that all Calls to F do not modify the memory pointed to by Arg,
// using alias analysis local to each caller of F.
-static bool isArgUnmodifiedByAllCalls(Argument *Arg, const LocationSize &Size,
+static bool isArgUnmodifiedByAllCalls(Argument *Arg,
FunctionAnalysisManager &FAM) {
for (User *U : Arg->getParent()->users()) {
@@ -499,9 +499,6 @@ static bool isArgUnmodifiedByAllCalls(Argument *Arg, const LocationSize &Size,
MemoryLocation Loc =
MemoryLocation::getForArgument(Call, Arg->getArgNo(), nullptr);
- // Refine the MemoryLocation Size using information from Loads of Arg.
- Loc = Loc.getWithNewSize(Size);
-
AAResults &AAR = FAM.getResult<AAManager>(*Call->getFunction());
// Bail as soon as we find a Call where Arg may be modified.
if (isModSet(AAR.getModRefInfo(Call, Loc)))
@@ -750,7 +747,7 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// If we can determine that no call to the Function modifies the memory region
// accessed through Arg, through alias analysis using actual arguments in the
// callers, we know that it is guaranteed to be safe to promote the argument.
- if (isArgUnmodifiedByAllCalls(Arg, LocationSize::precise(Offset), FAM))
+ if (isArgUnmodifiedByAllCalls(Arg, FAM))
return true;
// Otherwise, use alias analysis to check if the pointer is guaranteed to not
More information about the llvm-commits
mailing list