[PATCH] D115259: [AArch64][SVE] Lower vector.insert to predicated SEL

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm requested changes to this revision.
paulwalker-arm added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11009
+    SDValue ScalableVec1 = convertToScalableVector(DAG, VT, Vec1);
+    return DAG.getNode(ISD::VSELECT, DL, VT, PTrue, Vec0, ScalableVec1);
+  }
----------------
This is why the tests are wrong.  ISDOpcodes says `Select(COND, TRUEVAL, FALSEVAL)` and so `Vec0` and `ScalableVec1` need switching.  Might be worth writing a runable test just to verify this to yourself.


================
Comment at: llvm/test/CodeGen/AArch64/insert-subvector-res-legalization.ll:82
 ; CHECK-NEXT:    ushll v1.8h, v1.8b, #0
-; CHECK-NEXT:    st1h { z0.h }, p0, [sp]
-; CHECK-NEXT:    str q1, [sp]
-; CHECK-NEXT:    ld1h { z0.h }, p0/z, [sp]
-; CHECK-NEXT:    addvl sp, sp, #1
-; CHECK-NEXT:    ldr x29, [sp], #16 // 8-byte Folded Reload
+; CHECK-NEXT:    sel z0.h, p0, z0.h, z1.h
 ; CHECK-NEXT:    ret
----------------
Is this not backwards? `SEL` says "Read active elements from the first source vector and inactive elements from the second source vector" so the subvector should be the first source?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115259