[HLSL] Add the DXC matrix orientation flags #171550
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang-driver Author: Farzon Lotfi (farzonl) Changesfixes #58676
Full diff: https://github.com/llvm/llvm-project/pull/171550.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 3e6f7a2a430ff..fca84904326c9 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -826,6 +826,8 @@ def err_drv_target_variant_invalid : Error<
def err_drv_invalid_directx_shader_module : Error<
"invalid profile : %0">;
+def err_drv_dxc_invalid_matrix_layout : Error<
+ "cannot specify /Zpr and /Zpc together">;
def err_drv_dxc_missing_target_profile : Error<
"target profile option (-T) is missing">;
def err_drv_hlsl_unsupported_target : Error<
diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td
index e55146f0c7823..35fb9d5bd227a 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -9587,6 +9587,8 @@ class DXCJoinedOrSeparate<string name> : Option<["/", "-"], name,
KIND_JOINED_OR_SEPARATE>, Group<dxc_Group>,
Visibility<[DXCOption]>;
+def dxc_col_major : DXCFlag<"Zpc">, HelpText<"Pack matrices in column-major order">;
+def dxc_row_major : DXCFlag<"Zpr">, HelpText<"Pack matrices in row-major order">;
def dxc_no_stdinc : DXCFlag<"hlsl-no-stdinc">,
HelpText<"HLSL only. Disables all standard includes containing non-native compiler types and functions.">;
def dxc_Fo : DXCJoinedOrSeparate<"Fo">,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 542b70b3e9d4c..b0d1c994de160 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3723,6 +3723,10 @@ static void RenderHLSLOptions(const ArgList &Args, ArgStringList &CmdArgs,
if (!Args.hasArg(options::OPT_dxc_no_stdinc) &&
!Args.hasArg(options::OPT_nostdinc))
CmdArgs.push_back("-finclude-default-header");
+ if (Args.hasArg(options::OPT_dxc_col_major))
+ CmdArgs.push_back("-fmatrix-memory-layout=column-major");
+ if (Args.hasArg(options::OPT_dxc_row_major))
+ CmdArgs.push_back("-fmatrix-memory-layout=row-major");
}
static void RenderOpenACCOptions(const Driver &D, const ArgList &Args,
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 5d7221b8718a9..8f58b2f449a9a 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -412,6 +412,10 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
const OptTable &Opts = getDriver().getOpts();
+ if (Args.hasArg(options::OPT_dxc_col_major) &&
+ Args.hasArg(options::OPT_dxc_row_major))
+ getDriver().Diag(diag::err_drv_dxc_invalid_matrix_layout);
+
for (Arg *A : Args) {
if (A->getOption().getID() == options::OPT_dxil_validator_version) {
StringRef ValVerStr = A->getValue();
diff --git a/clang/test/Driver/hlsl_matrix_pack_order.hlsl b/clang/test/Driver/hlsl_matrix_pack_order.hlsl
new file mode 100644
index 0000000000000..f7b6902fd02d8
--- /dev/null
+++ b/clang/test/Driver/hlsl_matrix_pack_order.hlsl
@@ -0,0 +1,8 @@
+// RUN: %clang_dxc -T lib_6_7 -Zpr -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-ROW-MAJOR
+// CHECK-ROW-MAJOR: -fmatrix-memory-layout=row-major
+
+// RUN: %clang_dxc -T lib_6_7 -Zpc -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-COL-MAJOR
+// CHECK-COL-MAJOR: -fmatrix-memory-layout=column-major
+
+// RUN: not %clang_dxc -Tlib_6_7 -Zpr -Zpc -fcgl -Fo - %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISMATCH-MAJOR
+// CHECK-MISMATCH-MAJOR: cannot specify /Zpr and /Zpc together
|
tex3d
approved these changes
Dec 10, 2025
farzonl
commented
Dec 10, 2025
Comment on lines
3726
to
3729
| if (Args.hasArg(options::OPT_dxc_col_major)) | ||
| CmdArgs.push_back("-fmatrix-memory-layout=column-major"); | ||
| if (Args.hasArg(options::OPT_dxc_row_major)) | ||
| CmdArgs.push_back("-fmatrix-memory-layout=row-major"); |
Member
Author
There was a problem hiding this comment.
try and figure out how to replace this with
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 5d7221b8718a..11597b168f59 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -506,7 +510,18 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
A->claim();
continue;
}
-
+ if (A->getOption().getID() == options::OPT_dxc_col_major) {
+ DAL->AddSeparateArg(nullptr, Opts.getOption(options::OPT_fmatrix_memory_layout_EQ),
+ "column-major");
+ A->claim();
+ continue;
+ }
+ if (A->getOption().getID() == options::OPT_dxc_row_major) {
+ DAL->AddSeparateArg(nullptr, Opts.getOption(options::OPT_fmatrix_memory_layout_EQ),
+ "row-major");
+ A->claim();
+ continue;
+ }
DAL->append(A);
}
inbelic
approved these changes
Dec 11, 2025
Contributor
inbelic
left a comment
inbelic
left a comment
There was a problem hiding this comment.
LGTM, although I think we should use AddJoinedArg instead of AddSeparateArg.
farzonl
commented
Dec 11, 2025
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
fixes llvm#58676 - Make /Zpr and /Zpc turn on the -fmatrix-memory-layout= row-major and column-major flags - Add the new DXC driver flags to Options.td - Error in the HLSL toolchain when both flags are specified - Add the new error diagnostic to DiagnosticDriverKinds.td - propogate the flag via the Clang toolchain
…ptions. Then set it via HLSLToolChain::TranslateArgs.
farzonl
force-pushed
the
feature/dxc-orientation-flags-issue-58676
branch
from
9e5df35 to
e4ad3c3
Compare
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/18942 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #58676