[PATCH] D129282: [AArch64][SVE] Ensure PTEST operands have type nxv16i1

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Thanks @RosieSumpter for fixing this. Just left a new nits, but overall the changes look sensible to me.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:241
+// Returns true if newly defined lanes are known to be zeroed by construction.
+static bool hasZeroedOtherLanes(SDValue Op) {
+  switch (Op.getOpcode()) {
----------------
nit: isZeroingInactiveLanes ?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16185
+  // Ensure operands have type nxv16i1.
+  MVT WidenVT = MVT::nxv16i1;
+  SDValue ReinterpretOp = Op;
----------------
nit: Can you propagate `MVT::nxv16i1` to all uses of `WidenVT`, and remove `WidenVT`?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16186-16187
+  MVT WidenVT = MVT::nxv16i1;
+  SDValue ReinterpretOp = Op;
+  SDValue ReinterpretPg = Pg;
+  if (Op.getValueType() != WidenVT) {
----------------
nit: are `ReinterpertOp` and `ReinterpretPg` needed? I think you can just reassign `Op` and `Pg`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129282