[PATCH] D107157: [NFC] Clean up tests in test/Transforms/LoopVectorize/assume.ll
David Sherwood via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 30 08:06:32 PDT 2021
david-arm added a comment.
Hi @spatel, you make a valid point that the update script makes it a lot easier to add tests and so on. I completely agree. The problem I found was that it took me a long time to figure out what was actually being tested here that's all. For example, in `predicated_assume` we're actually testing that we *haven't* added the assume intrinsic, so I thought a CHECK-NOT line made it very obvious that's all.
In general, and this is nothing to do with your tests (which are good tests by the way!), I sometimes find with all the CHECK lines for vectoriser tests is that there is a high chance that your patch changed something that actually has nothing to do with what the test is testing, and it can also consume quite a bit of time trying to understand if you've genuinely broken something or not. I do see though that there are pros and cons either way!
If you prefer to leave the tests as they are that's fine by me. :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107157/new/
https://reviews.llvm.org/D107157
More information about the llvm-commits
mailing list