From d10cb42c7c7f55d06ed093c334cd82adad524730 Mon Sep 17 00:00:00 2001 From: Hari Limaye 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 1f9b546ed2999..428f809d3d45f 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 &ArgPartsVec) { + SmallVectorImpl &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 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(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(*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 PointerArgs; - for (Argument &I : F->args()) - if (I.getType()->isPointerTy()) - PointerArgs.push_back(&I); - if (PointerArgs.empty()) + SmallVector 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> 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 ArgParts; - if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts)) { + if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts, + ArgNotModified)) { SmallVector 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 63366ba998c7b..d52bfc3111779 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 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 428f809d3d45f..05b1bf9f3cee1 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(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(*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 &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 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(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(*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 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 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> 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 ArgParts; if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts, - ArgNotModified)) { + FAM)) { SmallVector 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 d52bfc3111779..ca757a165fa4b 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 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 05b1bf9f3cee1..8f3a603f914dd 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(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(*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 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 8f3a603f914dd..aaa9792c00249 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(*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 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 aaa9792c00249..a7e88f0479721 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 9db0f349bcf49f1c5c55bf4e58bfbdac641561bd Mon Sep 17 00:00:00 2001 From: Hari Limaye Date: Tue, 24 Sep 2024 21:30:35 +0000 Subject: [PATCH 6/6] Remove Size calculation --- llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp index a7e88f0479721..90e8c39e5a90d 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(*Call->getFunction()); // Bail as soon as we find a Call where Arg may be modified. if (isModSet(AAR.getModRefInfo(Call, Loc))) @@ -728,8 +725,7 @@ 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. This also computes the size of the - // memory region accessed through Arg. + // Make sure the parts are non-overlapping. int64_t Offset = ArgPartsVec[0].first; for (const auto &Pair : ArgPartsVec) { if (Pair.first < Offset) @@ -750,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 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