[PATCH] D69956: [AArch64][SVE] Integer reduction instructions pattern/intrinsics.

[Graham Hunter via Phabricator via llvm-commits]

huntergr added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:169
+    addRegisterClass(MVT::v1i8, &AArch64::FPR8RegClass);
+    addRegisterClass(MVT::v1i16, &AArch64::FPR16RegClass);
+
----------------
efriedma wrote:
> dancgr wrote:
> > efriedma wrote:
> > > Is this change necessary?  Making these legal has other effects I don't really want to think about.
> > Its necessary to legalize those types for FPR. I have ran all tests for code-gen and it did not seam to have any side effect. For NEON we are currently legalizing v16i8 for FPR8 and v8i16 for FPR16 in the same way.
> I just tried this and I see failures (if I enable this for all NEON targets, not just SVE).
We never needed to add those as legal types downstream. Instead, we use INSERT_SUBREG in output patterns.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10533
+
+    SDValue Temp = SDValue(DAG.getMachineNode(TargetOpcode::IMPLICIT_DEF, dl, OutputVT), 0);
+
----------------
This appears to be using the INSERT_SUBREG I mentioned above, but in C++ code. For this case I think it's better to use a tablegen pattern.

The extract will be needed for all element types though.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:5825
+
+  def : SVE_2_Op_Pat<v1i8, op, nxv16i1, nxv16i8, !cast<Instruction>(NAME # _B)>;
+  def : SVE_2_Op_Pat<v1i16, op, nxv8i1, nxv8i16, !cast<Instruction>(NAME # _H)>;
----------------
So this is where we implemented the INSERT_SUBREG pattern, like this:

```
  def : Pat<(v16i8 (op (nxv16i1 PPR3bAny:$Pg), (nxv16i8 ZPR8:$Zn))),
            (INSERT_SUBREG (v16i8 (IMPLICIT_DEF)), (!cast<Instruction>(NAME#_B) PPR3bAny:$Pg, ZPR8:$Zn), bsub)>;
```

Similarly for the other types and reduce multiclasses.

While we prefer to use `SVE_2_Op_Pat` and similar helpers where possible, sometimes we have to use more involved patterns in this file. Doing so avoids the need to add new legal types just to support the reductions.


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

https://reviews.llvm.org/D69956