[PATCH] D16623: Factor out UnrollAnalyzer to Analysis, and add unit tests for it.

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a reviewer: sanjoy.
sanjoy added a comment.
This revision now requires changes to proceed.

I think this change requires some reorganization before it can go in.  Please take a look at http://llvm.org/docs/CodingStandards.html


================
Comment at: include/llvm/Analysis/LoopUnrollAnalyzer.h:6
@@ +5,3 @@
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+
----------------
This is very unlike headers in the rest of LLVM -- have you looked at other headers in this directory?  For instance:

 - Headers should not open namespaces
 - License header is missing
 - Function bodies (e.g. `visitBinaryOperator`) should be in the cpp file

================
Comment at: include/llvm/Analysis/LoopUnrollAnalyzer.h:25
@@ +24,3 @@
+//   v = b[1]
+namespace {
+class UnrolledInstAnalyzer : private InstVisitor<UnrolledInstAnalyzer, bool> {
----------------
This should be in namespace llvm.

I'd say it is better to not even expose this as an `InstVisitor` but keep that an implementation detail; but that's debatable.


http://reviews.llvm.org/D16623