[PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit permutation & table lookup

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1226
+  // intrinsic currently, where NumVecs is always 2
+  unsigned NumVecs = 2;
+
----------------
nit: You can just as well inline this value now.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1229
+  // Form a REG_SEQUENCE to force register allocation.
+  SmallVector<SDValue, 4> Regs(N->op_begin() + 1, N->op_begin() + 1 + NumVecs);
+  SDValue RegSeq = createZTuple(Regs);
----------------
nit: given that we know NumVecs == 2, you can write `SmallVector<SDVDalue, 2>`.
nit: How about `N->ops().slice(1, 2)` ?
https://llvm.org/doxygen/classllvm_1_1ArrayRef.html#ace7bdce94e806bb8870626657630dab0


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1235
+  Ops.push_back(N->getOperand(NumVecs + 1));
+  ReplaceNode(N, CurDAG->getMachineNode(Opc, DL, VT, Ops));
+}
----------------
nit: Maybe just:
```ReplaceNode(N, CurDAG->getMachineNode(Opc, DL, VT, { RegSeq, N->getOperand(NumVecs + 1) });```


================
Comment at: llvm/test/CodeGen/AArch64/sve2-intrinsics-perm-tb.ll:9
+; CHECK-LABEL: tbl2_b:
+; CHECK: tbl z0.b, { z0.b, z1.b }, z2.b
+; CHECK-NEXT: ret
----------------
We should test this with operands that are not already consecutive. `%a` and `%b` will come in as `z0` and `z1` by definition of the calling convention.
By adding a `%dummy` in between `%a` and `%b`, you can check that a `mov` is inserted to ensure both registers are consecutive.


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

https://reviews.llvm.org/D74912