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

[Danilo Carvalho Grael via Phabricator via llvm-commits]

dancgr marked 5 inline comments as done.
dancgr added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:169
+    addRegisterClass(MVT::v1i8, &AArch64::FPR8RegClass);
+    addRegisterClass(MVT::v1i16, &AArch64::FPR16RegClass);
+
----------------
huntergr wrote:
> 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.
Will be removing this.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10533
+
+    SDValue Temp = SDValue(DAG.getMachineNode(TargetOpcode::IMPLICIT_DEF, dl, OutputVT), 0);
+
----------------
huntergr wrote:
> 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.
Will change the pattern according to the following comment, that way I can get rid of this custom lowering.


================
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)>;
----------------
huntergr wrote:
> 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.
I will update the solution for this. I will be putting that in a re-usable pattern and simplify the lowering.


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

https://reviews.llvm.org/D69956