Skip to content

scalar layout #145063

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.

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

Closed
wants to merge 3 commits into from
Closed

scalar layout #145063

wants to merge 3 commits into from

Conversation

s-perron
Copy link
Contributor

  • [HLSL][SPIRV] Allow large z value in numthreads
  • Implement DXC layout cli options.
  • [HLSL] Add option for VK layouts

s-perron added 3 commits June 19, 2025 13:56
The current validation checks for numthreads assume that the target is
DXIL so the version checks inadvertently issue error when targeting
SPIR-V.
We add the options to the driver, so that the one option that is being
implemented at this time can be used. Issue an error for the other
options.

When we start to implement the other options, we will have to figure out
how to pass which  option is active to clang.
@s-perron s-perron requested a review from Keenuts June 20, 2025 16:09
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Jun 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Steven Perron (s-perron)

Changes
  • [HLSL][SPIRV] Allow large z value in numthreads
  • Implement DXC layout cli options.
  • [HLSL] Add option for VK layouts

Full diff: https://github.com/llvm/llvm-project/pull/145063.diff

6 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+10)
  • (modified) clang/lib/Driver/ToolChains/HLSL.cpp (+20-1)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+5-2)
  • (added) clang/test/Driver/HLSL/dxc_fvk_layout.hlsl (+8)
  • (modified) clang/test/Driver/dxc_fspv_extension.hlsl (+2-2)
  • (modified) clang/test/SemaHLSL/num_threads.hlsl (+12-4)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0ffd8c40da7da..24ddffb8d6b1b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -9320,6 +9320,16 @@ def fspv_extension_EQ
       Group<dxc_Group>,
       HelpText<"Specify the available SPIR-V extensions. If this option is not "
                "specified, then all extensions are available.">;
+def fvk_use_dx_layout
+    : DXCFlag<"fvk-use-dx-layout">,
+      HelpText<"Use DirectX memory layout for Vulkan resources.">;
+def fvk_use_gl_layout : DXCFlag<"fvk-use-gl-layout">,
+                        HelpText<"Use strict OpenGL std140/std430 memory "
+                                 "layout for Vulkan resources.">;
+def fvk_use_scalar_layout
+    : DXCFlag<"fvk-use-scalar-layout">,
+      HelpText<"Use scalar memory layout for Vulkan resources.">;
+
 def no_wasm_opt : Flag<["--"], "no-wasm-opt">,
   Group<m_Group>,
   HelpText<"Disable the wasm-opt optimizer">,
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index dcc51e182924c..be59cc895667a 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -200,7 +200,7 @@ bool checkExtensionArgsAreValid(ArrayRef<std::string> SpvExtensionArgs,
   for (auto Extension : SpvExtensionArgs) {
     if (!isValidSPIRVExtensionName(Extension)) {
       Driver.Diag(diag::err_drv_invalid_value)
-          << "-fspv_extension" << Extension;
+          << "-fspv-extension" << Extension;
       AllValid = false;
     }
   }
@@ -330,6 +330,25 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
       A->claim();
       continue;
     }
+
+    if (A->getOption().getID() == options::OPT_fvk_use_dx_layout) {
+      // This is the only implemented layout so far.
+      A->claim();
+      continue;
+    }
+
+    if (A->getOption().getID() == options::OPT_fvk_use_scalar_layout) {
+      getDriver().Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+      A->claim();
+      continue;
+    }
+
+    if (A->getOption().getID() == options::OPT_fvk_use_gl_layout) {
+      getDriver().Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+      A->claim();
+      continue;
+    }
+
     DAL->append(A);
   }
 
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index b55f4fd786b58..9f39c077cea7a 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1033,12 +1033,15 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
 void SemaHLSL::handleNumThreadsAttr(Decl *D, const ParsedAttr &AL) {
   llvm::VersionTuple SMVersion =
       getASTContext().getTargetInfo().getTriple().getOSVersion();
+  bool IsDXIL = getASTContext().getTargetInfo().getTriple().getArch() ==
+                llvm::Triple::dxil;
+
   uint32_t ZMax = 1024;
   uint32_t ThreadMax = 1024;
-  if (SMVersion.getMajor() <= 4) {
+  if (IsDXIL && SMVersion.getMajor() <= 4) {
     ZMax = 1;
     ThreadMax = 768;
-  } else if (SMVersion.getMajor() == 5) {
+  } else if (IsDXIL && SMVersion.getMajor() == 5) {
     ZMax = 64;
     ThreadMax = 1024;
   }
diff --git a/clang/test/Driver/HLSL/dxc_fvk_layout.hlsl b/clang/test/Driver/HLSL/dxc_fvk_layout.hlsl
new file mode 100644
index 0000000000000..a796fbb8e052b
--- /dev/null
+++ b/clang/test/Driver/HLSL/dxc_fvk_layout.hlsl
@@ -0,0 +1,8 @@
+// No errors. Otherwise nothing observable.
+// RUN: %clang_dxc -fvk-use-dx-layout -spirv -Tlib_6_7 -### %s
+
+// RUN: not %clang_dxc -fvk-use-scalar-layout -spirv -Tlib_6_7 -### %s 2>&1 | FileCheck %s -check-prefix=SCALAR
+// SCALAR: error: the clang compiler does not support '-fvk-use-scalar-layout'
+
+// RUN: not %clang_dxc -fvk-use-gl-layout -spirv -Tlib_6_7 -### %s 2>&1 | FileCheck %s -check-prefix=GL
+// GL: error: the clang compiler does not support '-fvk-use-gl-layout' 
diff --git a/clang/test/Driver/dxc_fspv_extension.hlsl b/clang/test/Driver/dxc_fspv_extension.hlsl
index 0a9d321b8d95e..ff414ab6c0a6e 100644
--- a/clang/test/Driver/dxc_fspv_extension.hlsl
+++ b/clang/test/Driver/dxc_fspv_extension.hlsl
@@ -12,8 +12,8 @@
 
 // Check for the error message if the extension name is not properly formed.
 // RUN: not %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=TEST1 -fspv-extension=SPV_GOOD -fspv-extension=TEST2 2>&1 | FileCheck %s -check-prefix=FAIL
-// FAIL: invalid value 'TEST1' in '-fspv_extension'
-// FAIL: invalid value 'TEST2' in '-fspv_extension'
+// FAIL: invalid value 'TEST1' in '-fspv-extension'
+// FAIL: invalid value 'TEST2' in '-fspv-extension'
 
 // If targeting DXIL, the `-spirv-ext` should not be passed to the backend.
 // RUN: %clang_dxc -Tlib_6_7 -### %s 2>&1 | FileCheck %s -check-prefix=DXIL
diff --git a/clang/test/SemaHLSL/num_threads.hlsl b/clang/test/SemaHLSL/num_threads.hlsl
index b5f9ad6c33cd6..96200312bbf69 100644
--- a/clang/test/SemaHLSL/num_threads.hlsl
+++ b/clang/test/SemaHLSL/num_threads.hlsl
@@ -10,6 +10,8 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel5.0-compute -x hlsl -ast-dump -o - %s -DFAIL -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel4.0-compute -x hlsl -ast-dump -o - %s -DFAIL -verify
 
+// RUN: %clang_cc1 -triple spirv-pc-vulkan1.3-compute -x hlsl -ast-dump -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
+
 #if __SHADER_TARGET_STAGE == __SHADER_STAGE_COMPUTE || __SHADER_TARGET_STAGE == __SHADER_STAGE_MESH || __SHADER_TARGET_STAGE == __SHADER_STAGE_AMPLIFICATION || __SHADER_TARGET_STAGE == __SHADER_STAGE_LIBRARY
 #ifdef FAIL
 
@@ -88,24 +90,30 @@ int entry() {
 
 // Because these two attributes match, they should both appear in the AST
 [numthreads(2,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:90:2, col:18> 2 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 2 2 1
 int secondFn();
 
 [numthreads(2,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:94:2, col:18> 2 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 2 2 1
 int secondFn() {
   return 1;
 }
 
 [numthreads(4,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:100:2, col:18> 4 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 4 2 1
 int onlyOnForwardDecl();
 
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:100:2, col:18> Inherited 4 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> Inherited 4 2 1
 int onlyOnForwardDecl() {
   return 1;
 }
 
+#ifdef __spirv__ 
+[numthreads(4,2,128)]
+// CHECK-SPIRV: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:20> 4 2 128
+int largeZ();
+#endif 
+
 #else // Vertex and Pixel only beyond here
 // expected-error-re@+1 {{attribute 'numthreads' is unsupported in '{{[A-Za-z]+}}' shaders, requires one of the following: compute, amplification, mesh}}
 [numthreads(1,1,1)]

@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-hlsl

Author: Steven Perron (s-perron)

Changes
  • [HLSL][SPIRV] Allow large z value in numthreads
  • Implement DXC layout cli options.
  • [HLSL] Add option for VK layouts

Full diff: https://github.com/llvm/llvm-project/pull/145063.diff

6 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+10)
  • (modified) clang/lib/Driver/ToolChains/HLSL.cpp (+20-1)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+5-2)
  • (added) clang/test/Driver/HLSL/dxc_fvk_layout.hlsl (+8)
  • (modified) clang/test/Driver/dxc_fspv_extension.hlsl (+2-2)
  • (modified) clang/test/SemaHLSL/num_threads.hlsl (+12-4)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0ffd8c40da7da..24ddffb8d6b1b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -9320,6 +9320,16 @@ def fspv_extension_EQ
       Group<dxc_Group>,
       HelpText<"Specify the available SPIR-V extensions. If this option is not "
                "specified, then all extensions are available.">;
+def fvk_use_dx_layout
+    : DXCFlag<"fvk-use-dx-layout">,
+      HelpText<"Use DirectX memory layout for Vulkan resources.">;
+def fvk_use_gl_layout : DXCFlag<"fvk-use-gl-layout">,
+                        HelpText<"Use strict OpenGL std140/std430 memory "
+                                 "layout for Vulkan resources.">;
+def fvk_use_scalar_layout
+    : DXCFlag<"fvk-use-scalar-layout">,
+      HelpText<"Use scalar memory layout for Vulkan resources.">;
+
 def no_wasm_opt : Flag<["--"], "no-wasm-opt">,
   Group<m_Group>,
   HelpText<"Disable the wasm-opt optimizer">,
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index dcc51e182924c..be59cc895667a 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -200,7 +200,7 @@ bool checkExtensionArgsAreValid(ArrayRef<std::string> SpvExtensionArgs,
   for (auto Extension : SpvExtensionArgs) {
     if (!isValidSPIRVExtensionName(Extension)) {
       Driver.Diag(diag::err_drv_invalid_value)
-          << "-fspv_extension" << Extension;
+          << "-fspv-extension" << Extension;
       AllValid = false;
     }
   }
@@ -330,6 +330,25 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
       A->claim();
       continue;
     }
+
+    if (A->getOption().getID() == options::OPT_fvk_use_dx_layout) {
+      // This is the only implemented layout so far.
+      A->claim();
+      continue;
+    }
+
+    if (A->getOption().getID() == options::OPT_fvk_use_scalar_layout) {
+      getDriver().Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+      A->claim();
+      continue;
+    }
+
+    if (A->getOption().getID() == options::OPT_fvk_use_gl_layout) {
+      getDriver().Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
+      A->claim();
+      continue;
+    }
+
     DAL->append(A);
   }
 
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index b55f4fd786b58..9f39c077cea7a 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1033,12 +1033,15 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
 void SemaHLSL::handleNumThreadsAttr(Decl *D, const ParsedAttr &AL) {
   llvm::VersionTuple SMVersion =
       getASTContext().getTargetInfo().getTriple().getOSVersion();
+  bool IsDXIL = getASTContext().getTargetInfo().getTriple().getArch() ==
+                llvm::Triple::dxil;
+
   uint32_t ZMax = 1024;
   uint32_t ThreadMax = 1024;
-  if (SMVersion.getMajor() <= 4) {
+  if (IsDXIL && SMVersion.getMajor() <= 4) {
     ZMax = 1;
     ThreadMax = 768;
-  } else if (SMVersion.getMajor() == 5) {
+  } else if (IsDXIL && SMVersion.getMajor() == 5) {
     ZMax = 64;
     ThreadMax = 1024;
   }
diff --git a/clang/test/Driver/HLSL/dxc_fvk_layout.hlsl b/clang/test/Driver/HLSL/dxc_fvk_layout.hlsl
new file mode 100644
index 0000000000000..a796fbb8e052b
--- /dev/null
+++ b/clang/test/Driver/HLSL/dxc_fvk_layout.hlsl
@@ -0,0 +1,8 @@
+// No errors. Otherwise nothing observable.
+// RUN: %clang_dxc -fvk-use-dx-layout -spirv -Tlib_6_7 -### %s
+
+// RUN: not %clang_dxc -fvk-use-scalar-layout -spirv -Tlib_6_7 -### %s 2>&1 | FileCheck %s -check-prefix=SCALAR
+// SCALAR: error: the clang compiler does not support '-fvk-use-scalar-layout'
+
+// RUN: not %clang_dxc -fvk-use-gl-layout -spirv -Tlib_6_7 -### %s 2>&1 | FileCheck %s -check-prefix=GL
+// GL: error: the clang compiler does not support '-fvk-use-gl-layout' 
diff --git a/clang/test/Driver/dxc_fspv_extension.hlsl b/clang/test/Driver/dxc_fspv_extension.hlsl
index 0a9d321b8d95e..ff414ab6c0a6e 100644
--- a/clang/test/Driver/dxc_fspv_extension.hlsl
+++ b/clang/test/Driver/dxc_fspv_extension.hlsl
@@ -12,8 +12,8 @@
 
 // Check for the error message if the extension name is not properly formed.
 // RUN: not %clang_dxc -spirv -Tlib_6_7 -### %s -fspv-extension=TEST1 -fspv-extension=SPV_GOOD -fspv-extension=TEST2 2>&1 | FileCheck %s -check-prefix=FAIL
-// FAIL: invalid value 'TEST1' in '-fspv_extension'
-// FAIL: invalid value 'TEST2' in '-fspv_extension'
+// FAIL: invalid value 'TEST1' in '-fspv-extension'
+// FAIL: invalid value 'TEST2' in '-fspv-extension'
 
 // If targeting DXIL, the `-spirv-ext` should not be passed to the backend.
 // RUN: %clang_dxc -Tlib_6_7 -### %s 2>&1 | FileCheck %s -check-prefix=DXIL
diff --git a/clang/test/SemaHLSL/num_threads.hlsl b/clang/test/SemaHLSL/num_threads.hlsl
index b5f9ad6c33cd6..96200312bbf69 100644
--- a/clang/test/SemaHLSL/num_threads.hlsl
+++ b/clang/test/SemaHLSL/num_threads.hlsl
@@ -10,6 +10,8 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel5.0-compute -x hlsl -ast-dump -o - %s -DFAIL -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel4.0-compute -x hlsl -ast-dump -o - %s -DFAIL -verify
 
+// RUN: %clang_cc1 -triple spirv-pc-vulkan1.3-compute -x hlsl -ast-dump -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
+
 #if __SHADER_TARGET_STAGE == __SHADER_STAGE_COMPUTE || __SHADER_TARGET_STAGE == __SHADER_STAGE_MESH || __SHADER_TARGET_STAGE == __SHADER_STAGE_AMPLIFICATION || __SHADER_TARGET_STAGE == __SHADER_STAGE_LIBRARY
 #ifdef FAIL
 
@@ -88,24 +90,30 @@ int entry() {
 
 // Because these two attributes match, they should both appear in the AST
 [numthreads(2,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:90:2, col:18> 2 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 2 2 1
 int secondFn();
 
 [numthreads(2,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:94:2, col:18> 2 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 2 2 1
 int secondFn() {
   return 1;
 }
 
 [numthreads(4,2,1)]
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:100:2, col:18> 4 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> 4 2 1
 int onlyOnForwardDecl();
 
-// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:100:2, col:18> Inherited 4 2 1
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:18> Inherited 4 2 1
 int onlyOnForwardDecl() {
   return 1;
 }
 
+#ifdef __spirv__ 
+[numthreads(4,2,128)]
+// CHECK-SPIRV: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}} <line:{{[0-9]+}}:2, col:20> 4 2 128
+int largeZ();
+#endif 
+
 #else // Vertex and Pixel only beyond here
 // expected-error-re@+1 {{attribute 'numthreads' is unsupported in '{{[A-Za-z]+}}' shaders, requires one of the following: compute, amplification, mesh}}
 [numthreads(1,1,1)]

@s-perron s-perron closed this Jun 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants