[PATCH] D44001: [SCEV] Prove implications for SCEVUnknown Phis
Anna Thomas via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 26 13:59:22 PDT 2018
anna added a comment.
Comments inline. There's 2 dominator checks (dominates and isReachableFromEntry) that's done per phi operand. That seems like a good amount of compile time usage. There's also a isReachableFromEntry check within the `dominates` function. I cannot see a way to reduce these checks in your patch though.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:9586
+ // If both are Phis, then they should be from one basic block.
+ if (LPhi->getParent() != RPhi->getParent())
+ return false;
----------------
Pls separate out LPhi->getParent() into a var such as `CommonParent` and use here and below at line 9591.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:9603
+ return true;
+ } else if (LPhi) {
+ if (!properlyDominates(RHS, LPhi->getParent()))
----------------
I think you can extract out the logic here into another lambda because there's a lot of code duplication.
Really all we want to check using logic below and it can be extracted into `isImplied(PhiNode *Phi, const Scev *LHSOrRHS)`.
https://reviews.llvm.org/D44001
More information about the llvm-commits
mailing list