[SM68] Initial proposal for Wave Matrix #61

llvm-beanz · 2023-06-22T19:04:31Z

This proposal adds new WaveMatrix data types that support lane cooperative matrix multiplication.

A preview implementation of this proposal is available as part of the DXC 1.8.2306 preview release.

This proposal adds new WaveMatrix data types that support lane cooperative matrix multiplication. A preview implementation of this proposal is available as part of the DXC 1.8.2306 preview release.

pow2clk

Sorry for the slew of comments 😕

pow2clk · 2023-08-22T03:06:50Z

proposals/xxxx-wave-matrix.md

+
+These higher throughput matrix operations are required for optimal performance
+of many machine learning and image processing workloads. Adding native support
+to HLSL will enable high-performance matrix operations across all supported


It's minor, but I fear the notion of "supported" is vague here. It may be interpreted as all hardware that supports SM 6.8, which is probably not the case. My feeble attempt to recraft the sentence:

Adding support to HLSL will enable the high-performance of native matrix operations across all hardware with such support through Shader Model 6.8 drivers.

pow2clk · 2023-08-22T03:08:10Z

proposals/xxxx-wave-matrix.md

+WaveMatrix introduces new matrix templates to facilitate wave cooperative
+operations:
+
+```c++


It doesn't seem to make a lick of difference in this case, but ```hlsl is an option in github:

WaveMatrixLeft <TYPE_IN, M, N> ; // M x K

WaveMatrixLeft <TYPE_IN, M, N> ; // M x K

Maybe someday we'll get proper syntax highlighting?

FYI if you check the 3rd party grammars supported by Github's Linguist integration here, HLSL is listed as being supported by Tim's textmate grammar (so hlsl is a valid fenced code block and we just need to update that grammar to update it as the language changes). I would actually suggest having an "official" Microsoft-maintained grammar file that you can update as the compiler updates if it isn't too much trouble. It'd be super helpful for HLSL tool maintainers to have a more "official" syntax highlighter.

In this code block the HLSL highlighting is probably okay, but GitHub’s HLSL syntax highlighting doesn’t handle HLSL 2021 particularly well. The C++ mode syntax highlighting does much better IMO.

As an example here’s HLSL highlighted:

namespace detail { template <typename ElTy, int NRows, int NCols> class WaveMatrixBase { }; } // namespace detail

And the same code C++ highlighted:

namespace detail { template <typename ElTy, int NRows, int NCols> class WaveMatrixBase { }; } // namespace detail

I’m fine changing the simpler code samples to be HLSL highlighted, but I’d greatly prefer to keep the ones that have templates C++-mode. I had used C++ everywhere for consistency, but that isn’t strictly necessary.

Right, I think the thing to do is to fork the C++ tmLanguage file as a base, adding HLSL specific constructs/keywords, and then swap it as the official highlighting grammar for github (instructions for doing that are here. It'd be a shame for this to be fixed later and then have a bunch of HLSL snippets throughout the github ecosystem be tagged as C++ snippets into perpetuity I think!

pow2clk · 2023-08-22T05:11:50Z

proposals/xxxx-wave-matrix.md

+All WaveMatrix objects have a `Fill` method of the form `void Fill(ElTy Value)`
+where `ElTy` is the element type.
+
+The `Fill` method fills the matrix or matrix fragment with the provided value.


I think a more technical description would be "Assigns the given Value to every element in the matrix or matrix fragment".

pow2clk · 2023-08-22T05:13:39Z

proposals/xxxx-wave-matrix.md

+All wave threads must provide the same value or the result is undefined. All
+WaveMatrix objects have the same `Fill` method with the same behavior.
+
+### WaveMatrix Matrix Objects


I feel like the explanation of Fill above would be a bit more logical after this section

pow2clk · 2023-08-22T05:14:19Z

proposals/xxxx-wave-matrix.md

+### WaveMatrix Matrix Objects
+
+The code below approximately declares the base interface that WaveMatrix matrix
+objects implement.


Maybe add "the following sections will explain in detail what these methods do and what the parameters represent."

pow2clk · 2023-08-22T05:41:40Z

proposals/xxxx-wave-matrix.md

+The `WaveMatrixAccumulator::MultiplyAccumulate` method performs multiplication
+of the left and right arguments and adds the result back into the
+`WaveMatrixAccumulator`. This is a wave-level operation and cannot be used
+inside divergent control flow.


as above, what if I do?

pow2clk · 2023-08-22T05:43:58Z

proposals/xxxx-wave-matrix.md

+WaveMatrix intrinsics are defined to support quantization calculations.
+Including calculating a sum for the rows of the left matrix and a sum of the
+columns of the right matrix. The `WaveMatrixRightRowAcc` and
+`WaveMatrixLeftColAcc` fragment accumulators perform this operation.


Should they have "Fragment" in the name? I was initially confused why they took full matrices as parameters, but I see through the inheritance that they are fragment accumulators

I don't love the names of those types, but I think if we add "Fragment" to the name we're going to be getting dangerously close to a type name that can't fit on one line of code with reasonable line wrapping rules.

pow2clk · 2023-08-22T05:47:26Z

proposals/xxxx-wave-matrix.md

+#### Zero Point
+
+The following is the equation for matrix multiplication with zero point
+adjustment included:


This section feels out of place in between the definition of WaveMatrix Fragment Acuumulators and the description of their SumAccumulate method. I recognize that it is referenced in the section just after it, but if this is defining the multiply operations, perhaps it can go after the multiply method description and the next section can link back to it?

pow2clk · 2023-08-22T05:49:29Z

proposals/xxxx-wave-matrix.md

+
+$Z_*$ are constant zero points values
+
+#### Wave Matrix SumAccumulate


maybe "WaveMatrix Fragment SumAccumulate" to be consistent with the title of that section?

pow2clk · 2023-08-22T05:50:53Z

proposals/xxxx-wave-matrix.md

+#### Wave Matrix SumAccumulate
+
+The `SumAccumulate` methods accumulate the values of the argument matrix into
+the WaveMatrix fragment accumulator. The fragment WaveMatrix must have the same


Reading this and the description above the class, it's not clear to me what actually gets placed into the accumulator. Is it the sum of all elements in each row of a row-major matrix and the sum of all elements in each column of a column-major matrix?

Degerz · 2024-08-07T04:21:55Z

microsoft/DirectXShaderCompiler#6807

I assume that this proposal has been denied ?

llvm-beanz · 2024-08-07T17:40:48Z

microsoft/DirectXShaderCompiler#6807

I assume that this proposal has been denied ?

Denied is probably the wrong phrasing... In need of significant reworking.

This is definitely a feature we want, but the design proposed here has some pretty big limitations that we haven't had time to address. That resulted in us pausing development on it and not shipping it with the Shader Model 6.8 release.

We hope to refine this feature (in public) and include an updated version of it in the future.

[SM68] Initial proposal for Wave Matrix

4ad5a69

This proposal adds new WaveMatrix data types that support lane cooperative matrix multiplication. A preview implementation of this proposal is available as part of the DXC 1.8.2306 preview release.

llvm-beanz requested review from tex3d and pow2clk June 22, 2023 19:04

llvm-beanz added this to the Shader Model 6.8 milestone Jul 6, 2023

llvm-beanz assigned pow2clk and tex3d Jul 7, 2023

alan-baker mentioned this pull request Jul 13, 2023

[NNNN] Wave Matrix saturating accumulation #67

Open

alan-baker mentioned this pull request Jul 28, 2023

[SM??] Wave Matrix clarifications #72

Open

pow2clk reviewed Aug 22, 2023

View reviewed changes

llvm-beanz added 2 commits August 22, 2023 12:10

Renormalize line endings.

3cea383

Update based on review feedback.

eb479ce

pow2clk removed this from the Shader Model 6.8 milestone Jan 3, 2024

llvm-beanz added 2 commits January 16, 2024 13:40

Move to SM 6.9

f31db93

Add DXIL changes

08203e5

sudonatalie mentioned this pull request May 9, 2024

[Feature Request] [SPIR-V] WMMA support via VK_KHR_cooperative_matrix.. microsoft/DirectXShaderCompiler#6585

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SM68] Initial proposal for Wave Matrix #61

[SM68] Initial proposal for Wave Matrix #61

llvm-beanz commented Jun 22, 2023

pow2clk left a comment

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

jeremyong Aug 22, 2023

llvm-beanz Aug 22, 2023

jeremyong Aug 22, 2023 •

edited

Loading

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

llvm-beanz Aug 22, 2023

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

pow2clk Aug 22, 2023

Degerz commented Aug 7, 2024

llvm-beanz commented Aug 7, 2024


		$Z_*$ are constant zero points values

		#### Wave Matrix SumAccumulate

[SM68] Initial proposal for Wave Matrix #61

Are you sure you want to change the base?

[SM68] Initial proposal for Wave Matrix #61

Conversation

llvm-beanz commented Jun 22, 2023

pow2clk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremyong Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Degerz commented Aug 7, 2024

llvm-beanz commented Aug 7, 2024

jeremyong Aug 22, 2023 •

edited

Loading