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
588–591

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

590

mova?

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

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

2699

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.

2867

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.

2983

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 }

3317

Same comment as with sme2_mova_vec_to_tile_vg4_multi_base.

3430

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
2699

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
1431

immediate

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

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.

3305

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

3365

indent me

3390

indent me

3417

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.

3421

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