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

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 2 13:58:42 PDT 2015


sanjoy added inline comments.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:3781
@@ +3780,3 @@
+  auto IsAvailableOnEntry = [&](const SCEV *S, BasicBlock *BB) {
+    struct CheckAvailable {
+      bool TraversalDone = false;
----------------
joker.eph wrote:
> 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?
First of all, I'm not opposed to moving lifting the lambda outside if you think that'll help readability.  I don't remember seeing anything about this in the coding style, but I can certainly understand the sentiment. :)

My bias is towards keeping things in as restricted a scope as possible.  If a structure / helper function is useful only inside a function, then it should stay within that function, and not be leaked into the file scope etc.  IOW, if something is not generally useful, it should be generally available either (same reason why you'd want to put the struct in an anonymous namespace, and not inside the `llvm` namespace).  But this is just a bias (not a strong rational reason).


http://reviews.llvm.org/D13378





More information about the llvm-commits mailing list