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

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 5 14:18:58 PST 2016


sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

LGTM with minor suggestions, sorry for the delay.  I only casually skimmed through the logic, since this is supposed to be an NFC.


================
Comment at: lib/Analysis/LoopUnrollAnalyzer.cpp:19
@@ +18,3 @@
+
+namespace llvm {
+
----------------
Minor: usually .cpp files tend to `using namespace llvm;` instead of this.

================
Comment at: unittests/Analysis/UnrollAnalyzer.cpp:22
@@ +21,3 @@
+namespace {
+SmallVector<DenseMap<Value *, Constant *>, 16> SimplifiedValuesVector;
+unsigned TripCount = 0;
----------------
Minor: I'd just make these `static` and remove the anonymous namespace.  Instead, the anonymous namespace needs to be around `struct UnrollAnalyzerTest`.


http://reviews.llvm.org/D16623





More information about the llvm-commits mailing list