[PATCH] D13378: [SCEV] Recognize simple br-phi patterns

[Mehdi AMINI via llvm-commits]

joker.eph added a comment.

Hi Sanjoyd,

Thanks for the clarification on the Constant Select, isn't it something that could be handled as well? Especially with http://reviews.llvm.org/D13390 coming?
See inline some other comments.

Thanks!


================
Comment at: lib/Analysis/ScalarEvolution.cpp:3601
@@ -3600,3 +3600,3 @@
 /// createNodeForPHI - PHI nodes have two cases.  Either the PHI node exists in
 /// a loop header, making it a potential recurrence, or it doesn't.
 ///
----------------
Update the comment

================
Comment at: lib/Analysis/ScalarEvolution.cpp:3781
@@ +3780,3 @@
+  auto IsAvailableOnEntry = [&](const SCEV *S, BasicBlock *BB) {
+    struct CheckAvailable {
+      bool TraversalDone = false;
----------------
I have a question about the lambda and their size, and especially the second one with this inner structure. Usually I have seen in LLVM only very small lambda used in favor of static function for larger one. Also Structure helper are usually in an anonymous namespace. Is there any guideline on this?

================
Comment at: lib/Analysis/ScalarEvolution.cpp:3872
@@ +3871,3 @@
+        IsAvailableOnEntry(getSCEV(RHS), PN->getParent()))
+      return createNodeForSelect(PN, Cond, LHS, RHS);
+  }
----------------
What about renaming `createNodeForSelect` into `createNodeForSelectOrPhi`? And also update the comment on top of the definition for `createNodeForSelect`.


http://reviews.llvm.org/D13378