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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen 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);
----------------
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`.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128665