[PATCH] D106561: [AArch64] Optimise min/max lowering in ISel

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7108
+  if ((VT != MVT::v1i64 && VT != MVT::v2i64) ||
+      useSVEForFixedLengthVectorVT(VT, /*OverrideNEON=*/true)) {
+    switch (Opcode) {
----------------
david-arm wrote:
> I think you might be able to just do this:
> 
>   if (useSVEForFixedLengthVectorVT(VT, /*OverrideNEON=*/true)) {
> 
> instead?
Hi @Rin, apologies please ignore my previous comment about changing the if statement. I see now we also want scalable vector types to fall into the block too!


================
Comment at: llvm/test/CodeGen/AArch64/vecreduce-umax-legalization.ll:90
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:   mov     x8, v0.d[1]
-; CHECK-NEXT:   fmov    x9, d0
-; CHECK-NEXT:   cmp     x9, x8
-; CHECK-NEXT:   csel    x0, x9, x8, hi
-; CHECK-NEXT:   ret
+; CHECK-NEXT:    ext v1.16b, v0.16b, v0.16b, #8
+; CHECK-NEXT:    cmhi d2, d0, d1
----------------
dmgreen wrote:
> david-arm wrote:
> > Hi @Rin, at first glance this code looks like it might be worse than before? I realise the instruction count is the same, but I wonder if the cost of 'ext' might be higher?
> Why would the cost of an ext be higher?
@dmgreen it might not be! I just wondered if ext was worse than the mov that's all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106561