[PATCH] D17904: [LoopUnroll] Convert some existing tests to unit-tests.

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 11 17:11:28 PST 2016


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

Hi Michael,

I'm not familiar with the details of `UnrolledInstAnalyzer`, but this LGTM overall with the nits fixed.


================
Comment at: test/Transforms/LoopUnroll/full-unroll-heuristics-cmp.ll:7
@@ -6,3 @@
-; We should be able to propagate constant data through comparisons.
-; For example, in this test we have a load, which becomes constant after
-; unrolling, making comparison with 0 also known to be 0 (false) - and that
----------------
Should these comments be preserved in some form in the new tests?  Or do you think that now that we're doing whitebox testing, these comments are obvious?

================
Comment at: unittests/Analysis/UnrollAnalyzer.cpp:256
@@ +255,3 @@
+  EXPECT_TRUE(I1 != SimplifiedValuesVector[5].end());
+  EXPECT_EQ(dyn_cast<ConstantInt>((*I1).second)->getZExtValue(), 0U);
+}
----------------
I think here you need a `cast<>` not a `dyn_cast<>`, since you're not actually checking if `dyn_cast<>` returned a null.  Same comment on other `dyn_cast<>` s in the patch.


http://reviews.llvm.org/D17904





More information about the llvm-commits mailing list