[PATCH] D44001: [SCEV] Prove implications for SCEVUnknown Phis

[Anna Thomas via Phabricator via llvm-commits]

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