[PATCH] D128665: [AArch64] Make nxv1i1 types a legal type for SVE.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:6657
+  assert(InVT.isScalableVector() == NVT.isScalableVector() &&
          "cannot modify scalable vectors in this way");
   SDLoc dl(InOp);
----------------
sdesmalen wrote:
> efriedma wrote:
> > How are these changes related?  I would have thought that if you're making v1i1 legal, that would avoid triggering any target-independent legalization infrastructure.
> They're needed for the existing `<vscale x 1 x i1>` test-case in `llvm/test/CodeGen/AArch64/sve-select.ll`:
> 
>   select <vscale x 1 x i1> %p, <vscale x 1 x i64> %a, <vscale x 1 x i64> %dst
> 
> Before `%p` was assumed to be widened. But now, only `%a` and `%b` need widening, so it calls `ModifyToType` to widen `<vscale x 1 x i1> %p` as well to `<vscale x 1 x i1>`. This leads to the extra `uzp1`.
Oh, that makes sense.  And I guess that explains why the uzp1 shows up in sel_nxv1i64.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-pred-creation.ll:48
+; CHECK: ptrue p0.d
+; CHECK-NEXT: punpklo p0.h, p0.b
+; CHECK-NEXT: ret
----------------
sdesmalen wrote:
> efriedma wrote:
> > I don't think, in general, we guarantee that padding bits of SVE predicates are zeroed.  So the punpklo should be unnecessary.
> I wasn't sure about this to be honest. If we can assume that the other lanes are always zeroed, then we can avoid having to always unpack the predicate when using this value (rather than when creating it). For example, the patterns for `AArch64ptest` don't first mask the predicate with a `ptrue <required element type>` and the instruction always tests each bit. That made me question if it was safe to assume the other lanes are always known to be zeroed.
As far as I know, nothing has changed since we last discussed this in https://reviews.llvm.org/D74471 .  Unless I'm forgetting something...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128665