[ArgPromotion] Perform alias analysis on actual arguments of Calls #106216
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -485,11 +485,36 @@ 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, | ||
| FunctionAnalysisManager &FAM) { | ||
| 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; | ||
|
|
||
| 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. | ||
| if (isModSet(AAR.getModRefInfo(Call, Loc))) | ||
| return false; | ||
| } | ||
|
|
||
| // All Users are Calls which do not modify the Arg. | ||
|
Contributor
There was a problem hiding this comment. This comment, the one on line 492, the one on line 488/499, and the function name all pretty much say the same thing. Maybe delete the ones in the function body
Contributor
Author
There was a problem hiding this comment. Removed redundant comments in Address review comments 2 |
||
| 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, | ||
| FunctionAnalysisManager &FAM) { | ||
| // Quick exit for unused arguments | ||
| if (Arg->use_empty()) | ||
| return true; | ||
|
|
@@ -716,10 +741,16 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR, | |
| 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. | ||
|
|
||
| // 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)) | ||
| 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. | ||
|
|
@@ -846,7 +877,8 @@ static Function *promoteArguments(Function *F, FunctionAnalysisManager &FAM, | |
| // If we can promote the pointer to its value. | ||
| SmallVector<OffsetAndArgPart, 4> ArgParts; | ||
|
|
||
| if (findArgParts(PtrArg, DL, AAR, MaxElements, IsRecursive, ArgParts, | ||
| FAM)) { | ||
| SmallVector<Type *, 4> Types; | ||
| for (const auto &Pair : ArgParts) | ||
| Types.push_back(Pair.second.Ty); | ||
|
|
||
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
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.
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.
Why is this checking for
CallInstrather thanCallBase? This should also work for invokes, right?And I think then you could also replace it with an assert, as the function only being used as CallBase callee is a precondition of trying the transform in the first place.
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.
Apologies, I was perhaps a bit trigger happy with committing this. I have put up #110335 to address this.