[PATCH] D114713: [AArch64][SVE][NEON] Add NEON-SVE-Bridge intrinsics

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1325
       setOperationAction(ISD::MLOAD, VT, Custom);
+      setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom);
     }
----------------
MattDevereau wrote:
> paulwalker-arm wrote:
> > Can you extract this into its own patch as it's really not relevant to the rest of the patch and is currently missing tests.  Presumably `llvm/test/CodeGen/AArch64/sve-insert-vector.ll` needs updating?
> i've been adding some tests to assert this block of code. i've got tests for `insert(vscale x n x bfloat, n x bfloat, idx)` and `insert(vscale x n x bfloat, vscale x n x bfloat, idx)`.
> the n = 4 and n = 8 tests are fine, but n = 2 for `insert(vscale x 2 x bfloat, 2 x bfloat, idx)`  fails an assertion. i've had a quick poke around but haven't seen an obvious reason why its failing, should I worry about this and spend more time on it or just submit the tests i've already got for `4bf16` and `8bf16`?
Obviously it would be nice for all combinations to work but that's not something you have to fix if it's not directly affecting what you need.

I've checked and it seems `2 x half` doesn't work out of the box either so it sounds reasonable to me for your new `bfloat` handling to mirror the existing supported `half` use cases only.


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

https://reviews.llvm.org/D114713