[PATCH] D83811: [AArch64][SVE] Add support for trunc to <vscale x N x i1>.

Wed Jul 15 04:25:48 PDT 2020

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.

LGTM as long as we can replace the two instances of ElementCount.Min as suggested.

================
Comment at: llvm/test/CodeGen/AArch64/sve-trunc.ll:77-144
+define <vscale x 4 x i1> @trunc_i64toi1_split(<vscale x 4 x i64> %in) {
+; CHECK-LABEL: trunc_i64toi1_split:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    ptrue p0.d
+; CHECK-NEXT:    and z1.d, z1.d, #0x1
+; CHECK-NEXT:    and z0.d, z0.d, #0x1
+; CHECK-NEXT:    cmpne p1.d, p0/z, z1.d, #0
----------------
efriedma wrote:
> paulwalker-arm wrote:
> > efriedma wrote:
> > > paulwalker-arm wrote:
> > > > Based on the assumption that these tests are really testing the CONCAT_VECTORS functionality, is it worth naming them as such (perhaps also move into sve-shuffles.ll or something)?
> > > Well, it's testing the truncate itself, and the legalization of scalable truncates, and yes, also the concat_vectors functionality.  I can rename them, sure; maybe trunc_i64toi1_split_op_and_concatenate_result.  I think the location is fine, since the way it's expressed in IR is just a plain truncate.
> > I guess I was just wondering if you would have still written these tests if there was a route to test CONCAT_VECTORS in isolation (of which I understand there currently isn't).  If you're happy with the location then I'm happy with their current names.
> We need these tests to test the legalization aspect, which can't be tested any other way.
Yep, understood.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83811