[PATCH] D79598: [AArch64][SVE] Add patterns for VSELECT of immediates.

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi @efriedma ,

thanks you for working on this.

> This doesn't handle the case where the operands are reversed, and the condition is freely invertible.

Do you mean cases like `select <vscale x 2 x i1> %p, <vscale x 2 x i64> zeroinitializer,  <vscale x 2 x i64> %three`? If that the case, the "merging" version of CPY , where `%three` is allocated to the dest register, should do it? `CPY <Zd>.<T>, <Pg>/M, <R><n|SP>`.

Kind regards,

Francesco



================
Comment at: llvm/test/CodeGen/AArch64/sve-vselect-imm.ll:9
+; CHECK-NEXT:    ret
+%three = shufflevector <vscale x 16 x i8> insertelement (<vscale x 16 x i8> undef, i8 3, i32 0), <vscale x 16 x i8> zeroinitializer, <vscale x 16 x i32> zeroinitializer
+%sel = select <vscale x 16 x i1> %p, <vscale x 16 x i8> %three, <vscale x 16 x i8> zeroinitializer
----------------
The immediate is a signed immediate - could you please test negative values too?

Nit: the name `%three` doesn't seem to be related to anything other than the value `3` of some of the tests... could you change it to  something more generic, like `%vec`, or even leave it as `%0`?


================
Comment at: llvm/test/CodeGen/AArch64/sve-vselect-imm.ll:74-111
+define <vscale x 8 x i16> @sel3_16(<vscale x 8 x i1> %p) {
+; CHECK-LABEL: sel3_16:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #513
+; CHECK-NEXT:    mov z0.h, w8
+; CHECK-NEXT:    mov z1.h, #0 // =0x0
+; CHECK-NEXT:    sel z0.h, p0, z0.h, z1.h
----------------
Can you do the same "invalid values" tests for when the scaling shift is 0? For example -130 and 130. 

Nit: adding the token `_invalid_imm_values` in the names of the functions would help a lot identifying what you are testing here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79598