[PATCH] D68814: [LV] Allow assume calls in predicated blocks.

[Ayal Zaks via Phabricator via llvm-commits]

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

Agree that it's better to merge the new conditional assume tests with the existing unconditional assume tests in the same file.

Thanks for making the changes!



================
Comment at: llvm/test/Transforms/LoopVectorize/predicate-assume.ll:2
+; REQUIRES: asserts
+; RUN: opt -loop-vectorize -force-vector-width=4 -debug -S %s 2>&1 | FileCheck %s
+
----------------
fhahn wrote:
> gilr wrote:
> > fhahn wrote:
> > > I've not added a test with calls to assume in the loop header here, because we already have them in llvm/test/Transforms/LoopVectorize/X86/assume.ll.
> > It would be better to add the new predicated cases to the existing assume.ll.
> > (Not sure why it resides under X86 - seems it could instead be target independent and force vectorization, but that can be done separately)
> I am not sure. I personally prefer smaller, more targeted test files, but if you think having a larger test file is better I'll merge them.
The above forthcoming comment emphasizes that these tests complement each other: “I've not added a test with calls to assume in the loop header here, because we already have them in llvm/test/Transforms/LoopVectorize/X86/assume.ll”.

Quoting the end of https://llvm.org/docs/TestingGuide.html#writing-new-regression-tests:
“Put related tests into a single file rather than having a separate file per test. Check if there are files already covering your feature and consider adding your code there instead of creating a new file.”


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68814/new/

https://reviews.llvm.org/D68814