[PATCH] D25464: [NFC] Loop Versioning for LICM
Evgeny Astigeevich via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 11 04:21:35 PDT 2016
eastig added inline comments.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:161
AU.addRequired<ScalarEvolutionWrapperPass>();
- AU.addRequired<TargetLibraryInfoWrapperPass>();
AU.addPreserved<AAResultsWrapperPass>();
----------------
TargetLibraryInfo is not used in the code.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:169
- TLI(nullptr), LAA(nullptr), LAI(nullptr), Changed(false),
- Preheader(nullptr), CurLoop(nullptr), CurAST(nullptr),
LoopDepthThreshold(LVLoopDepthThreshold),
----------------
Removed unused fields or made them function scoped instead of class scoped.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:176
+private:
AliasAnalysis *AA; // Current AliasAnalysis information
----------------
Made internal data private.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:305
- DEBUG(dbgs() << " Alias tracker type safety failed!\n");
- return false;
}
----------------
It's strange checking. It was questioned in https://reviews.llvm.org/D9151. An answer was:
> Actually this is a pre-condition in LICM’s “promoteLoopAccessesToScalars” where it expects all pointers in alias should have same type.
> To confirm same behaviour we added this check.
Yes, type checking is needed for AliasSet when all pointers must alias. This is done by LICM. In case of AliasSet whose pointers may alias we can have aliased pointers of different types, e.g. char * and char ** which might be aliased and they have different types.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:510
return false;
- Changed = false;
// Get Analysis information.
----------------
Converted to be function scoped.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:512
// Get Analysis information.
- LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
----------------
Converted to be function scoped.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:515
SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
- DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
- TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
----------------
eastig wrote:
> Removed as unused.
Converted to be function scoped.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:515-516
SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
- DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
- TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
LAA = &getAnalysis<LoopAccessLegacyAnalysis>();
----------------
Removed as unused.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:522
- // Get the preheader block.
- Preheader = L->getLoopPreheader();
- // Initial allocation
----------------
Removed as unused.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:524
- // Initial allocation
- CurAST = new AliasSetTracker(*AA);
----------------
This is not safe. Replaced with unique_ptr.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:567
INITIALIZE_PASS_DEPENDENCY(ScalarEvolutionWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
INITIALIZE_PASS_END(LoopVersioningLICM, "loop-versioning-licm",
----------------
Removed as unused.
https://reviews.llvm.org/D25464
More information about the llvm-commits
mailing list