This is an archive of the discontinued LLVM Phabricator instance.

[AArch64]SME2 MOV Instructions
ClosedPublic

Authored by CarolineConcatto on Oct 18 2022, 1:31 AM.

Details

Summary

This patch adds the assembly/disassembly for the following instructions:

MOVA (array to vector, four registers): Move four ZA single-vector groups to four vector registers.

(array to vector, two registers): Move two ZA single-vector groups to two vector registers.
(tile to vector, four registers): Move four ZA tile slices to four vector registers.
(tile to vector, single): Move ZA tile slice to vector register.
(tile to vector, two registers): Move two ZA tile slices to two vector registers.
(vector to array, four registers): Move four vector registers to four ZA single-vector groups.
(vector to array, two registers): Move two vector registers to two ZA single-vector groups.
(vector to tile, four registers): Move four vector registers to four ZA tile slices.
(vector to tile, single): Move vector register to ZA tile slice.
(vector to tile, two registers): Move two vector registers to two ZA tile slices.

The reference can be found here:

https://developer.arm.com/documentation/ddi0602/2022-09

It add more sizes for Matrix Operand:
MatrixOp8 and MatrixOp16
two implicit operands uimm0s2range and uimm0s4range.
and uimm1s2range that are immediates

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 18 2022, 1:31 AM
CarolineConcatto requested review of this revision.Oct 18 2022, 1:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 18 2022, 1:31 AM
Matt added a subscriber: Matt.Oct 21 2022, 12:39 PM
paulwalker-arm added inline comments.Nov 3 2022, 11:41 AM
llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td
434–437

Should these not be MOVA/MOV? the register suffix points to the direction of travel.

436

mova?

llvm/lib/Target/AArch64/SMEInstrFormats.td
2246

Please add comments to all the base instruction classes that reference their encoding groups.

2269

To be honest I don't see any value to this class, especially as it only contains a single InstAlias. I think the aliases will be more readable if just emitted directly.

2437

The vg2 case didn't do this so it's fine but for sme2_mova_vec_to_tile_vg4_multi_base please can you pass in the 3-bit opc and then use the {0, ?, ?} trick.

2553

Can sme2_mova_vec_to_array_vg24_multi take a 5-bit opcode and use { 0, ?,.. and then the vg4 instance will be {1, ?, ?, ?, 0 }

2887

Same comment as with sme2_mova_vec_to_tile_vg4_multi_base.

3000

Same comment as with sme2_mova_vec_to_array_vg2_multi.

CarolineConcatto marked 7 inline comments as done.
  • Address review comments
llvm/lib/Target/AArch64/SMEInstrFormats.td
2269

We created precendent for that when we created for sme1
multiclass sme_vector_to_tile_aliases<

I try to remove the alias class, but one class like this:

defm : sme2_mova_vec_to_tile_or_array_aliases<1, !cast<Instruction>(NAME # _B),
                                              !if(v, TileVectorOpV8,
                                                     TileVectorOpH8),
                                              MatrixIndexGPR32Op12_15,
                                              uimm3s2range,  ZZ_b_mul_r,
                                              "mov">;

Would become 2, one for Horizontal and another for Vertical:

def : InstAlias<mnemonic # "\t$ZAd[$Rs, $imm], $Zn",
                (!cast<Instruction>(NAME # _B) TileVector8H:$ZAd, rv_ty:$Rs, index_ty:$imm, ZZ_b_mul_r:$Zn), 1>;
def : InstAlias<mnemonic # "\t$ZAd[$Rs, $imm], $Zn",
                (!cast<Instruction>(NAME # _B) TileVector8V:$ZAd, rv_ty:$Rs, index_ty:$imm, ZZ_b_mul_r:$Zn), 1>;

We will have to duplicate the number of alias.
And I cannot pass the types (TileVector8H/TileVector8V, TileVector16H/TileVector16V/...)in the previous class :

multiclass sme2_mova_vec_to_tile_vg2_multi<string mnemonic>{
 defm _H : sme2_mova_vec_to_tile_vg2_multi_base<0b0, mnemonic>;
 defm _V : sme2_mova_vec_to_tile_vg2_multi_base<0b1, mnemonic>;
}

Because it depends on the size.

Unless you know some tricks in tablegen to solve that problem.

paulwalker-arm accepted this revision.Nov 7 2022, 7:36 AM

A couple of suggestions but otherwise looks good. I think my op3:op4:op5 comment will help with your follow-on work but I'm also happy if you want to delay that until you need the support (i.e. when you add MOVAZ).

llvm/lib/Target/AArch64/AArch64InstrFormats.td
1396

immediate

llvm/lib/Target/AArch64/SMEInstrFormats.td
2269

I see, thanks for explaining. Perhaps the H/V hierarchy is upside down but given this matches the precedent let's stick with what you've got.

2823

Please can this be Inst{7-5} = op;? then pass in {0, ?, ?} and {?, ?, ?} accordingly.

2883

indent me

2908

indent me

2935

I'm being picky but I think the following better matches the documentation. However, I understand this might be excessively verbose so it's up to you which you prefer.

let Inst{31-15} = 0b11000000;
let Inst{23-22} = 0b00; //op0
let Inst{21-19} = 0b000;
let Inst{18} = 0b1; // op1
let Inst{17} = 0b1;
let Inst{16-15} = 0b00; // op2

Same goes for sme2_mova_vec_to_array_vg24_multi.

2939

It'll better reflect the encoding group if you make op3:op4:op5 be the passed in opcode. I know there's commonality across some of the bits but it makes it easier to match the documentation and it'll just work when you come to add MOVAZ.

This revision is now accepted and ready to land.Nov 7 2022, 7:36 AM
This revision was automatically updated to reflect the committed changes.
CarolineConcatto marked 4 inline comments as done.
llvm/test/MC/AArch64/SME2/mova-diagnostics.s