[PATCH] D66413: [ARM} Add support for MVE vmaxv

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1064
     return false;
+  case Instruction::ICmp:
   case Instruction::Add:
----------------
samtebbs wrote:
> samparker wrote:
> > samtebbs wrote:
> > > samparker wrote:
> > > > Is this actually needed for codegen?
> > > It is, as otherwise the smax and umax vector reductions aren't generated and can't be selected on.
> > As in, the reductions get expanded somewhere and your tests would fail?
> Indeed, the tests fail and we get markedly worse codegen.
> 
> ```
> 65c65,70
> < 	vmaxv.u32	r2, q0
> ---
> > 	vmov.f32	s5, s3
> > 	vmax.u32	q0, q0, q1
> > 	vmov.32	r2, q0[1]
> > 	vdup.32	q1, r2
> > 	vmax.u32	q0, q0, q1
> > 	vmov	r2, s0
> ```
Ok, cheers. Then I expect Dave is right that the vmin could now get generated and will then assert because it can't be selected...?


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

https://reviews.llvm.org/D66413