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

[Eli Friedman via Phabricator via llvm-commits]

efriedma marked 2 inline comments as done.
efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2567
   // if we're trying to split it at all. assert() that's true, just in case.
-  assert(!(NumElements & 1) && "Splitting vector, but not in half!");
+  assert(!(NumElements.Min & 1) && "Splitting vector, but not in half!");
 
----------------
paulwalker-arm wrote:
> Since this is now an ElementCount this could be 
> ```assert(ElementCount / 2 && ....```
> which already asserts the division is lossless.
> 
We do that division a bit later in the code.

I could drop the assertion, I guess.


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83811