[PATCH] D85855: [SVE] Lower fixed length vector integer SMIN/SMAX

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-minmax.ll:24
+;
+; FMAXNM
+;
----------------
SMAX


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-minmax.ll:30
+; CHECK-LABEL: smax_v8i8:
+; CHECK: smax v{{[0-9]}}.8b, v{{[0-9]}}.8b, v{{[0-9]}}.8b
+; CHECK: ret
----------------
Has something happened here meaning you could not match the expect register usage?

Same goes for the other 64/128-bit tests.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-minmax.ll:37
+; Don't use SVE for 128-bit vectors.
+define void @smax_v16i8(<16 x i8>* %a, <16 x i8>* %b) #0 {
+; CHECK-LABEL: smax_v16i8:
----------------
For the 128-bit tests you can pass the data in as parameters and return the result directly.

I can see smin_v2i32 and smin_v4i32 are good so just need to ensure the other 64/128-bit tests are consistent with those.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-minmax.ll:306-334
+define <1 x i64> @smax_v1i64(<1 x i64> %op1, <1 x i64> %op2) #0 {
+; CHECK-LABEL: smax_v1i64:
+; CHECK: fmov [[OP2:x[0-9]+]], {{d[0-9]+}}
+; CHECK-NEXT: fmov [[OP1:x[0-9]+]], {{d[0-9]+}}
+; CHECK-NEXT: cmp [[OP1]], [[OP2]]
+; CHECK-NEXT: csel [[RES:x[0-9]+]], [[OP1]], [[OP2]], gt
+; CHECK-NEXT: fmov {{d[0-9]+}}, [[RES]]
----------------
I hit a similar problem with i64 based multiplies.  LowerToPredicatedOp now has an OverrideNEON parameter that allows the use of SVE instructions even for NEON sized vectors (although only enabled when wide vectors are enabled until we build up enough confidence to make it the default).

You'll also need a couple of extra setOperations lines, just search for "These operations are not supported on NEON but SVE can do them"


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-int-minmax.ll:411
+;
+; FMINNM
+;
----------------
SMIN


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85855