Updating D87981: [X86] AMX programming model prototype.
Fixed some clang format issue.

LGTM

LGTM

LGTM.

LGTM. @MaskRay Do you have time to review?

LGTM. Better to wait for Philip accept.

This looks like it requires more thorough discussions, doesn't it? We will also need discussions with the binutils side. We requested a GCC compiler driver option long ago but we do not reach a consensus (https://gcc.gnu.org/legacy-ml/gcc/2020-01/msg00358.html). IMHO the approval was in haste. With all due respect, I think I've seen such hasty approval (without any mentioning why such decisions are justifiable) and hasty commits in this area before several times, so I'll click "Request Changes" just in case of an accidental commit.

LGTM

I've created D77549 which uses AArch64's udot instruction to compute the result of multiplies on 4x4 tiles. To do so, first a tiled loop nest is created that iterates over the columns, rows and the inner dimension. In the inner loop, 4x4 tiles are loaded, multiplied (using the dot product) and accumulated. After the inner loop, the final result of the 4x4 tile is stored. The main reason I went for AArch64's udot is that I can easily run it, but IIUC the VNNI instructions are very similar, they just allow processing of larger tiles.

Please note that the patch is a bit rough around the edges and we currently it not clear how to specify 'multiply 8 bit operands, accumulate in 32 bit result' nature of those instructions; we will have to extend the llvm.matrix.multiply definition for that I think. But it should be enough for you to be able to get started with getting something working for VNNI. Please let me know if you have any questions or encounter any problems, either in the discussion for D77549 or email.

Cheers,
Florian

We used something similar internally successfully. If you are interested, I could share infrastructure to create code that applies smaller building blocks (like fast 2x2 multiplication) to lower multiplies on larger matrixes.

I have the similar question on how to lower matrix intrinsics to some HW specific intrinsics/instruction. For example, X86 have the AVX512_VNNI feature (https://software.intel.com/sites/landingpage/IntrinsicsGuide/#expand=39,5370,5361,364,142,139,2210&text=vnni). It can perform dot product computation. But after matrix intrinsic is lowered to vector, it seems difficult to transform the vector operation to AVX512_VNNI intrinsic/instruction.

LGTM

LGTM

LGTM.

LGTM

LGTM. Better to hold for few days to have Philips and MaskRay review as well.

This is enabled only when -Os is specified?

LGTM

@reames
I'm a little bit concerned about the performance impact of this patch. Do you have any data for the patch? Is there any performance penalty and how much code size is reduced?

The patch LGTM. I'd like to see the patch land before the llvm release.

Ping @ruiu

@ruiu,
Can we submit the patch?

In D71442#1789052, @pengfei wrote:

Ping.

I've gone as far as writing a rough draft of textual assembler support. If folks agree this is helpful, I'll abandon this patch, and post one in that direction. Any high level suggestions as to syntax? I see two major options, but definitely welcome suggestions w.r.t. naming/semantics/etc...

Option 1

.boundary_align 4
jmp foo

and

.boundary_align 4
.bundle_lock
test ...
jcc foo
.bundle_lock

Option 2

Option 1

.boundary_align 4
jmp foo
.end_boundary_align

and

.boundary_align 4
test ...
jcc foo
.end_boundary_align

p.s. If anyone has a better name than "boundary align" please suggest it. That's not a good name; it's just a placeholder.

In D70157#1773180, @reames wrote:

Recording something so I don't forget it when we get back to the prefix padding version. The write up on the bundle align mode stuff mentions a concerning memory overhead for the feature. Since the basic implementation techniques are similar, we need to make sure we assess the memory overhead of the prefix padding implementation. See https://www.chromium.org/nativeclient/pnacl/aligned-bundling-support-in-llvm for context. I don't believe this is likely to be an issue for the nop padding variant.

I'm currently preparing the patches on the clang side in addition to an update to cfe-dev. Please stay tuned and we would really appreciate any feedback there.

In the original RFC, we sketched the C/C++ support we envisioned using builtins. A simple example that loads two 4x4 matrixes, multiplies them, adds a third matrix to the result and stores the it can be found in the code below. Our initial proposal is quite stripped down and intended to be exposed to end users via something like a C++ matrix wrapper class.

typedef float m4x4_t __attribute__((matrix_type(4, 4)));
 
 
void f(m4x4_t *a, m4x4_t *b, m4x4_t *c, m4x4_t *r) {
  *r = __builtin_matrix_add(__builtin_matrix_multiply(*a, *b), *c);
}

Interesting. Do you have any patch for the C/C++ frontend? What does the C/C++ code look like?

In D67210#1659439, @RKSimon wrote:

rL371078 should fix this

In D67210#1659230, @RKSimon wrote:

Please abandon this, it isn't a valid solution to the issue (raised at PR43227). I have a WIP fix that will address this correctly.

@anton-afanasyev, I have no objection. Thank you for the effort to improve the performance.

Update the patch corresponding to Reid's comments.

@anton-afanasyev
Hi,
Do you have any performance data for the patch? I'd like to know what benchmark has performance gain with your patch. https://reviews.llvm.org/D64394 fixed perlbench regression, but I wonder what the performance gain do we achieve with the 2 patch?

@anton-afanasyev
Hi,
Did you look into the SPEC cpu2017/500.perlbench_r issue? There is some significant performance drop on X86 with the patch. I ask you to revert the patch first, and when the SPEC2017 regression is fixed, we can submit the patch again. How do you think?

In D60437#1462731, @hans wrote:

Okay, let's not worry too much about the test case, the patch seems obviously good.

Do you have commit access, or would you like me to commit for you?