[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