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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to