[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