Skip to content

[Matrix] Add a row\col major toggle in the clang driver #167628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+124 −0
Open

[Matrix] Add a row\col major toggle in the clang driver #167628

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fd63d4c
[Matrix] Add a row\col major toggle in the clang driver
farzonl Nov 12, 2025
e2f0a2d
add new documentation to address pr concerns
farzonl Nov 12, 2025
49a8726
Address pr comments, drop the -mllvm flags make this just about memor…
farzonl Nov 14, 2025
8cb8930
Merge branch 'main' into featue/issue-167621
farzonl Nov 17, 2025
fa843ed
Update comments in CompilerInvocation.cpp
farzonl Nov 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/LanguageExtensions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,11 @@ The matrix type extension supports explicit casts. Implicit type conversion betw
i = static_cast<matrix_5_5<int>>(d);
}

The matrix type extension will support column and row major layouts. The flag
to change this behavior is `-fmatrix-default-layout` used like so
`-fmatrix-default-layout=column-major` for column major and like so
`-fmatrix-default-layout=row-major` for row major.

Half-Precision Floating Point
=============================

Expand Down
5 changes: 5 additions & 0 deletions clang/docs/MatrixTypes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,11 @@ part of the draft specification.
The elements of a value of a matrix type are laid out in column-major order
without padding.

To change the default order to row major use the `-fmatrix-default-layout` flag.
This flag supports two flag argument values either `column-major` or `row-major`
used like so `-fmatrix-default-layout=column-major`.` This flag controls the
memory layout of matrix types.

We propose to provide a Clang option to override this behavior and allow
contraction of those operations (e.g. *-ffp-contract=matrix*).

Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ New Compiler Flags
- New option ``-fsanitize-debug-trap-reasons=`` added to control emitting trap reasons into the debug info when compiling with trapping UBSan (e.g. ``-fsanitize-trap=undefined``).
- New options for enabling allocation token instrumentation: ``-fsanitize=alloc-token``, ``-falloc-token-max=``, ``-fsanitize-alloc-token-fast-abi``, ``-fsanitize-alloc-token-extended``.
- The ``-resource-dir`` option is now displayed in the list of options shown by ``--help``.
- New option ``-fmatrix-default-layout`` added to control the memory layout of Clang matrix types. (e.g. ``-fmatrix-default-layout=column-major`` or ``-fmatrix-default-layout=row-major``).

Lanai Support
^^^^^^^^^^^^^^
Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/Options/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4642,6 +4642,11 @@ def fenable_matrix : Flag<["-"], "fenable-matrix">, Group<f_Group>,
HelpText<"Enable matrix data type and related builtin functions">,
MarshallingInfoFlag<LangOpts<"MatrixTypes">, hlsl.KeyPath>;

def fmatrix_default_layout_EQ : Joined<["-"], "fmatrix-default-layout=">,
HelpText<"Set default matrix layout (row-major or column-major)">,
Values<"row-major,column-major">,
Group<f_Group>;

defm raw_string_literals : BoolFOption<"raw-string-literals",
LangOpts<"RawStringLiterals">, Default<std#".hasRawStringLiterals()">,
PosFlag<SetTrue, [], [], "Enable">,
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5692,6 +5692,17 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fenable-matrix");
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-enable-matrix");
// Only handle default layout if matrix is enabled
if (const Arg *A =
Args.getLastArg(options::OPT_fmatrix_default_layout_EQ)) {
StringRef Val = A->getValue();
if (Val == "row-major" || Val == "column-major") {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back(Args.MakeArgString("-matrix-default-layout=" + Val));
Copy link
Contributor

@cofibrant cofibrant Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if my understanding is correct, I think we don't want these lines. Moreover, I think the flag should really be called -fmatrix-memory-layout (or something similar) to distinguish it from the LLVM flag -matrix-default-layout which isn't talking about memory layout.

Copy link
Contributor

@cofibrant cofibrant Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then, yes, we shouldn't have an option for row-major until we've implemented the intrinsics.

Copy link
Member Author

@farzonl farzonl Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so you want no association of the clang driver flag and the llvm flag? Drop all the -mllvm -matrix-default-layout=* CmdArgs additions?

Copy link
Contributor

@cofibrant cofibrant Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hoping Florian chimes in but, yes, I think these flags are orthogonal

Copy link
Member Author

@farzonl farzonl Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think long term this could get confusing because it will create the posibility for someone to set -fmatrix-memory-layout=row-major and then do -mllvm -matrix-default-layout=column-major. And what I was trying to solve here was to have one place in the driver to keep all these options in sync.

Copy link
Member Author

@farzonl farzonl Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cofibrant I've dropped all the -mllvm -matrix-default-layout=* specific changes. Instead all I'm doing now is using the existing flag to set a langOpt that I will use for a few other prs I plan to put out after this change merges. I still do think we will want one flag that will control everything, but that isn't necessary for me to proceed with my changes.

} else {
D.Diag(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
}
}
}

CodeGenOptions::FramePointerKind FPKeepKind =
Expand Down
38 changes: 38 additions & 0 deletions clang/test/Driver/fmatrix-default-layout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// RUN: %clang --target=x86_64-linux-gnu -fenable-matrix -fmatrix-default-layout=column-major %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-COL-MAJOR
// CHECK-COL-MAJOR: -fenable-matrix
// CHECK-COL-MAJOR: -mllvm
// CHECK-COL-MAJOR: -enable-matrix
// CHECK-COL-MAJOR: -mllvm
// CHECK-COL-MAJOR: -matrix-default-layout=column-major

// RUN: %clang --target=x86_64-linux-gnu -fenable-matrix -fmatrix-default-layout=row-major %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ROW-MAJOR
// CHECK-ROW-MAJOR: -fenable-matrix
// CHECK-ROW-MAJOR: -mllvm
// CHECK-ROW-MAJOR: -enable-matrix
// CHECK-ROW-MAJOR: -mllvm
// CHECK-ROW-MAJOR: -matrix-default-layout=row-major

// RUN: not %clang --target=x86_64-linux-gnu -fenable-matrix -fmatrix-default-layout=error-major %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR-MAJOR
// CHECK-ERROR-MAJOR: error: invalid value 'error-major' in '-fmatrix-default-layout=error-major'

// RUN: %clang --target=x86_64-linux-gnu -fmatrix-default-layout=column-major %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-COL-MAJOR-DISABLED
// CHECK-COL-MAJOR-DISABLED-NOT: -fenable-matrix
// CHECK-COL-MAJOR-DISABLED-NOT: -mllvm
// CHECK-COL-MAJOR-DISABLED-NOT: -enable-matrix
// CHECK-COL-MAJOR-DISABLED-NOT: -mllvm
// CHECK-COL-MAJOR-DISABLED-NOT: -matrix-default-layout=column-major

// RUN: %clang --target=x86_64-linux-gnu -fmatrix-default-layout=row-major %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ROW-MAJOR-DISABLED
// CHECK-ROW-MAJOR-DISABLED-NOT: -fenable-matrix
// CHECK-ROW-MAJOR-DISABLED-NOT: -mllvm
// CHECK-ROW-MAJOR-DISABLED-NOT: -enable-matrix
// CHECK-ROW-MAJOR-DISABLED-NOT: -mllvm
// CHECK-ROW-MAJOR-DISABLED-NOT: -matrix-default-layout=row-major

// RUN: %clang --target=x86_64-linux-gnu -fenable-matrix %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MATRIX-ENABLED
// CHECK-MATRIX-ENABLED: -fenable-matrix
// CHECK-MATRIX-ENABLED: -mllvm
// CHECK-MATRIX-ENABLED: -enable-matrix
// CHECK-MATRIX-ENABLED-NOT: -mllvm
// CHECK-MATRIX-ENABLED-NOT: -matrix-default-layout=row-major
// CHECK-MATRIX-ENABLED-NOT: -matrix-default-layout=column-major
Copy link
Member Author

@farzonl farzonl Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we would just have defaults stored in a LangOpt? So there woundn't be any need to have this flag if it wasn't specified.

if thats not right then I can change the default to be -mllvm -matrix-default-layout=column?

Loading