[0035] Specify a way to transpose Thread Scope matrices on load and constraint loads to A-type Matrices.

[anupamachandra]

This PR fixes PR#787.
Changes:

1. Extends the MatrixLayoutEnum to contain Transpose types that specify that the matrices will be transposed on load. This applies only to Thread scope optimal layout matrices. InterlockedAccumulate will continue to support only OuterProductOptimal layout for thread scope matrix, and not the transpose variant.
2. Also, we were missing a restriction on the Thread scope matrix loads, that they are only useful for A-type matrices as the only user of these loads are the Multiply(Matrix,Vector)/MultiplyAdd(Matrix,Vector) operations which only accept A Matrices. That constraint is added as well.

[llvm-beanz]

This sounds like we need some changes to the metadata and PSV0 to ensure that we are surfacing whether or not transpose is used.

[anupamachandra]

Yep, I can add a Layout field, but before that a couple of questions. Currently we have,

struct MatrixUse {
  uint32_t Dimensions[3]; // M, N, K
  uint8_t Scope;
  uint8_t OperandType;
  uint8_t ResultType;
  uint8_t RESERVED; // unused but reserved for padding/alignment.
  uint32_t Flags; // do we need this?
};

It seems to capture all the required details to validate the Matrix x Matrix Multiply operation, but not for Matrix x Vector operations, OuterProduct and the recently ratified VectorAccumulate. Should we add new structs to capture the relevant validation information for those?

struct MatrixVectorOpsUse {
uint8_t InputVecType;
uint8_t MatrixType;
bool MatrixIsTranposed;
uint8_t BiasType;
}

and a similar structs for OuterProdAcc and VectorAcc. If this seems ok, I'll add these to the PR.

[llvm-beanz]

The right approach might be to make a bigger change. Something like:

struct MatrixUseBase {
  uint8_t Scope;
  uint8_t OperandType;
}

struct MatrixMatrixUse : public MatrixUseBase {
  uint8_t ResultType;
  uint8_t RESERVED; // unused but reserved for padding/alignment.
  uint32_t Flags; // do we need this?
  uint32_t Dimensions[3]; // M, N, K
};

struct MatrixVectorUse : public MatrixUseBase{
   uint8_t InputVecType;
   uint8_t BiasType;
   uint32_t Flags; // 0x0001 - IsTransposed
   uint32_t Dimensions[2]; // M, N
   uint32_t Unused; // Only exists to make use struc
};

static_assert(sizeof(MatrixMatrixUse) == sizeof(MatrixVectorUse), "These should match!");

This allows us to have a single list of uses that are slightly different.

@jenatali & @JoeCitizen, this will have impact on the runtime side of things. Do you have thoughts?

[llvm-beanz]

Tagging @tex3d too, since a related topic came up in a meeting a few minutes ago.

[jenatali]

@llvm-beanz your proposal sounds good to me, yeah.

[anupamachandra]

@tex3d If you are ok with the above, I'll make the changes to the PR.

[tex3d]

More extensive changes than these are needed. Leave the PSV0 updates to me, and I should have a PR up for these later today.

[tex3d]

Forgot to leave follow-up comment. PR for PSV0/RDAT updates is here:
#832

[llvm-beanz]

I think this PR just needs a rebase and updating the godbolt link.

[damyanp]

@anupamachandra - can you get this ready to merge please?

Plan is to update the DXC implementation to match this for the preview (@hekota to put change together) by adding the new enum entries.

[anupamachandra]

Yep, will do.

[anupamachandra]

@anupamachandra - can you get this ready to merge please?

Plan is to update the DXC implementation to match this for the preview (@hekota to put change together) by adding the new enum entries.

All the changes are in. This is ready to merge.