[PATCH] D136091: [AArch64] SME2 multi-vec unpack and ZIP two and four registers

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added a comment.

Hey Paul, 
I also pushed FRINT instructions to be in this class



================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:1820
 
-class sme2_fp_cvt_vg4_multi<string mnemonic, bits<3> op>
-    : I<(outs ZZZZ_s_mul_r:$Zd), (ins ZZZZ_s_mul_r:$Zn),
+class sme2_zip_cvt_vg4_multi<bits<2> sz, bits<2> op1, bits<3> op2,
+                             bit op3, RegisterOperand first_vector_ty,
----------------
paulwalker-arm wrote:
> CarolineConcatto wrote:
> > paulwalker-arm wrote:
> > > paulwalker-arm wrote:
> > > > Have you reached the point where this class can just represent `SME2 Multi-vector - SVE Constructive Unary`?  Perhaps not all instructions within that encoding group fit but looking at this new definition of `Inst`, which I very much like by the way, suggests that is what this class effectively represents. So you just need to change a few opcodes to match the documentation.
> > > Actually looking at the register layouts perhaps this is not quite so clear cut, but I do think it's worth investigating to see if there's an options for more reuse.  If you decide the current class is still the best route then I do think streaming the opcode across three variables is not great.
> > I looked before trying to merge them, but the registers sizes change between the group of instructions.
> > 
> > I reduced from 4 to 3 set of bit. The problem is that for cvt we need 3 bits to select the instructions, and for zip it doesn't, the same 3 bits  need to be set because they are also the size.
> Sure I get the register sizes are different so we cannot have a single register class but it looks like we could have a smaller number of base classes based on register sizes?  For example a class which represents the "SME2 Multi-vector - SVE Constructive Unary" encoding group can support all instructions where both registers are 3 bit. So:
>   SME2 multi-vec FP to int convert four registers
>   SME2 multi-vec int to FP four registers
>   SME2 multi-vec quadwords ZIP four registers
>   SME2 multi-vec ZIP four registers
>   SME2 multi-vec FRINT four registers
> 
> Likewise and class similar to the above but where the registers are 4bit can be used for:
>   SME2 multi-vec FP to int convert two registers
>   SME2 multi-vec int to FP two registers
>   SME2 multi-vec FRINT two registers
> 
> So with these 2 base classes you cover half of the groups within this parent encoding group. It starts to be a case of diminishing returns after this but even then "SME2 multi-vec FP down convert two registers" and "SME2 multi-vec int down convert two registers" can be grouped together.
> 
> 
I could not do what you asked. So I refactored the entire class.
Now I have a main class sme2_sve_constructive_unary
 and many other small classes  derived from sme2_sve_constructive_unary.
The Zd and Zn size are different in every class that I had to create.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136091