MattDevereau marked an inline comment as done.
MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1325
       setOperationAction(ISD::MLOAD, VT, Custom);
+      setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom);
     }
----------------
paulwalker-arm wrote:
> 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.
Resolved this in 4244f95cc6ce73ab38fbb91929a0888309f3ca8d


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