[PATCH] D95039: [SVE] Add support for scalable vectorization of loops with selects and cmps

[Dave Green via Phabricator via llvm-commits]

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

The code changes look simple enough, LGTM.

Up to you if you want to try and reduce the test boilerplate too.



================
Comment at: llvm/test/Analysis/CostModel/sve-cmpsel.ll:9
+; Check icmp for a legal integer vector.
+define <vscale x 4 x i1> @cmp_nxv4i32(<vscale x 4 x i32> %a, <vscale x 4 x i32> %b) {
+; CHECK-LABEL: 'cmp_nxv4i32'
----------------
david-arm wrote:
> dmgreen wrote:
> > david-arm wrote:
> > > Here I've deliberately tried to avoid an explosion of test cases, so for the legal and illegal variants I used different element types.
> > Cost model checks don't really need valid inputs, they can just use undef. It makes adding a lot of them much simpler. See something like llvm/test/Analysis/CostModel/ARM/reduce-smax.ll or llvm/test/Analysis/CostModel/X86/arith-fma.ll. Even though they are adding many (sub-) architectures, they are still managable.
> Thanks for the suggestion @dmgreen!
I would combine all these into less functions to reduce all this boilerplate. Maybe one function for the cmp's and one for the sel's? Or maybe split out more into the legal/illegal types, like you have commented.  They don't need to actually use the output of the instruction.

It makes adding lots of test much easier to check, but up to you what you think.


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

https://reviews.llvm.org/D95039