[PATCH] D28724: Use getLoopLatch in place of isLoopSimplifyForm

Xin Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 17 10:41:05 PST 2017

trentxintong added a comment.

In https://reviews.llvm.org/D28724#648400, @MatzeB wrote:

> In https://reviews.llvm.org/D28724#648397, @MatzeB wrote:
> > Why was this implemented as a unittest? Wouldn't this work just as good by inserting some debug prints and then using lit+opt+FileCheck to test?
> Admittedly the same is true for many of the other tests in unittests/Analysis. Most of the times I just don't see the benefits of using unittests over lit+FileCheck, they are usually a lot harder to understand and debug.

One problem I can see with LIT test here is that getLoopID() is not a transformation by itself, it depends on other transformations to call/use it. Would not using a LIT test for that tranformation making the testing a little bit more fragile/unpredictable, i.e. what if the transformation later is changed not to call this API or call it in a different way.  Testing this way is a bit more specific I feel.



More information about the llvm-commits mailing list