[PATCH] D63693: [Polly][NFC][ScopBuilder] Move buildAliasChecks to ScopBuilder
Michael Kruse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 26 00:27:57 PDT 2019
Meinersbur added a comment.
In D63693#1554880 <https://reviews.llvm.org/D63693#1554880>, @domada wrote:
> Please let me know if such big patch is acceptable. I have moved all functions referenced only by `buildAliasChecks` function to ScopBuilder class.
Yes, if all changes have a common topic, in this case moving the alias check functionality. Even if we wanted tom move them, I don't think `hasFeasibleRuntimeContext` and `lookupBasePtrAccess` are specific to this functionality.
Could you change the title to reflect the topic, such as "Move buildAliasChecks and its implementing methods to ScopBuilder"?
================
Comment at: polly/include/polly/ScopBuilder.h:35-38
+ using AliasGroupTy = SmallVector<MemoryAccess *, 4>;
+
+ /// A vector of alias groups.
+ using AliasGroupVectorTy = SmallVector<AliasGroupTy, 4>;
----------------
AFICS these types are only used in private functions, i.e. could be private themselves.
================
Comment at: polly/include/polly/ScopInfo.h:2035
- /// Return the access for the base ptr of @p MA if any.
- MemoryAccess *lookupBasePtrAccess(MemoryAccess *MA);
-
----------------
I think this accessor might still be useful after havind constructed a SCoP on should stay.
================
Comment at: polly/include/polly/ScopInfo.h:2502
- /// @returns True if the optimized SCoP can be executed.
- bool hasFeasibleRuntimeContext() const;
-
----------------
This function might also be useful after the SCoP has been constructed and should stay.
================
Comment at: polly/include/polly/ScopInfo.h:2585
/// Return all alias groups for this SCoP.
- const MinMaxVectorPairVectorTy &getAliasGroups() const {
- return MinMaxAliasGroups;
- }
+ MinMaxVectorPairVectorTy &getAliasGroups() { return MinMaxAliasGroups; }
----------------
I don't think modifying an object's internals through a get-accessor is a good idea.
================
Comment at: polly/lib/Analysis/ScopBuilder.cpp:2381
scop->simplifyContexts();
- if (!scop->buildAliasChecks(AA)) {
+ if (!buildAliasChecks(AA, ORE)) {
LLVM_DEBUG(dbgs() << "Bailing-out because could not build alias checks\n");
----------------
After being moved to ScopBuilder, `buildAliasChecks` can just use `this->AA`. Similarly, as a uitility object `ORE` can be made a member of `ScopBuilder` so it's not needed to be passed every time.
Repository:
rPLO Polly
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63693/new/
https://reviews.llvm.org/D63693
More information about the llvm-commits
mailing list