Skip to content

Conversation

s-perron
Copy link
Contributor

@s-perron s-perron commented Jun 9, 2025

The HLSL driver currently defaults the triple to an unversioned os and
subarch when targeting SPIR-V. This means the SPIR-V backend decides the
default value. That is not a great option because a change the backend
could cause a change in Clang.

Now that we want to choose the default we need to consider the best
option. DXC currently defaults to Vulkan1.0. We are planning on not
supporting Vulkan1.0 in the Clang HLSL compiler because it is newer
versions of Vulkan are commonly supported on nearly all hardware, so
users do not use it.

Since we have to change from DXC anyway, we are using VK1.3. It has been
out long enough to be commonly available, and the initial implementation
of SPIR-V features for HLSL are assuming Vulkan 1.3.

The HLSL driver currently defaults the triple to an unversioned os and
subarch when targeting SPIR-V. This means the SPIR-V backend decides the
default value. That is not a great option because a change the backend
could cause a change in Clang.

Now that we want to choose the default we need to consider the best
option. DXC currently defaults to Vulkan1.0. We are planning on not
supporting Vulkan1.0 in the Clang HLSL compiler because it is newer
versions of Vulkan are commonly supported on nearly all hardware, so
users do not use it.

Since we have to change from DXC anyway, we are using VK1.3. It has been
out long enough to be commonly available, and the initial implementation
of SPIR-V features for HLSL are assuming Vulkan 1.3.
@s-perron s-perron requested a review from Keenuts June 9, 2025 14:29
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 9, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Steven Perron (s-perron)

Changes

The HLSL driver currently defaults the triple to an unversioned os and
subarch when targeting SPIR-V. This means the SPIR-V backend decides the
default value. That is not a great option because a change the backend
could cause a change in Clang.

Now that we want to choose the default we need to consider the best
option. DXC currently defaults to Vulkan1.0. We are planning on not
supporting Vulkan1.0 in the Clang HLSL compiler because it is newer
versions of Vulkan are commonly supported on nearly all hardware, so
users do not use it.

Since we have to change from DXC anyway, we are using VK1.3. It has been
out long enough to be commonly available, and the initial implementation
of SPIR-V features for HLSL are assuming Vulkan 1.3.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+12-14)
  • (modified) clang/test/Driver/dxc_spirv.hlsl (+1-1)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 73ff7757c3b04..af0c4f2012f30 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1596,28 +1596,26 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
       A->claim();
 
       if (Args.hasArg(options::OPT_spirv)) {
+        const llvm::StringMap<llvm::Triple::SubArchType> ValidTargets = {
+            {"vulkan1.2", llvm::Triple::SPIRVSubArch_v15},
+            {"vulkan1.3", llvm::Triple::SPIRVSubArch_v16}};
         llvm::Triple T(TargetTriple);
-        T.setArch(llvm::Triple::spirv);
-        T.setOS(llvm::Triple::Vulkan);
 
-        // Set specific Vulkan version if applicable.
+        // Set specific Vulkan version. Default to vulkan1.3.
+        auto TargetInfo = ValidTargets.find("vulkan1.3");
         if (const Arg *A = Args.getLastArg(options::OPT_fspv_target_env_EQ)) {
-          const llvm::StringMap<llvm::Triple::SubArchType> ValidTargets = {
-              {"vulkan1.2", llvm::Triple::SPIRVSubArch_v15},
-              {"vulkan1.3", llvm::Triple::SPIRVSubArch_v16}};
-
-          auto TargetInfo = ValidTargets.find(A->getValue());
-          if (TargetInfo != ValidTargets.end()) {
-            T.setOSName(TargetInfo->getKey());
-            T.setArch(llvm::Triple::spirv, TargetInfo->getValue());
-          } else {
+          TargetInfo = ValidTargets.find(A->getValue());
+          if (TargetInfo == ValidTargets.end()) {
             Diag(diag::err_drv_invalid_value)
                 << A->getAsString(Args) << A->getValue();
           }
           A->claim();
         }
-
-        TargetTriple = T.str();
+        if (TargetInfo != ValidTargets.end()) {
+          T.setOSName(TargetInfo->getKey());
+          T.setArch(llvm::Triple::spirv, TargetInfo->getValue());
+          TargetTriple = T.str();
+        }
       }
     } else {
       Diag(diag::err_drv_dxc_missing_target_profile);
diff --git a/clang/test/Driver/dxc_spirv.hlsl b/clang/test/Driver/dxc_spirv.hlsl
index e6624e5f1b3f6..65c9018dc54c5 100644
--- a/clang/test/Driver/dxc_spirv.hlsl
+++ b/clang/test/Driver/dxc_spirv.hlsl
@@ -3,7 +3,7 @@
 // RUN: %clang_dxc -T cs_6_0 -spirv -fspv-target-env=vulkan1.3 -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-VULKAN13
 // RUN: not %clang_dxc -T cs_6_0 -spirv -fspv-target-env=vulkan1.0 -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
 
-// CHECK: "-triple" "spirv-unknown-vulkan-compute"
+// CHECK: "-triple" "spirv1.6-unknown-vulkan1.3-compute"
 // CHECK-SAME: "-x" "hlsl"
 
 // CHECK-VULKAN12: "-triple" "spirv1.5-unknown-vulkan1.2-compute"

Copy link
Contributor

@Keenuts Keenuts left a comment

Choose a reason for hiding this comment

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

a nit, otherwise LGTM

s-perron and others added 2 commits June 10, 2025 14:26
Co-authored-by: Nathan Gauër <github@keenuts.net>
@s-perron s-perron merged commit b3db0c6 into llvm:main Jun 11, 2025
7 checks passed
@s-perron s-perron deleted the vk1.3 branch June 11, 2025 14:30
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
The HLSL driver currently defaults the triple to an unversioned os and
subarch when targeting SPIR-V. This means the SPIR-V backend decides the
default value. That is not a great option because a change the backend
could cause a change in Clang.

Now that we want to choose the default we need to consider the best
option. DXC currently defaults to Vulkan1.0. We are planning on not
supporting Vulkan1.0 in the Clang HLSL compiler because it is newer
versions of Vulkan are commonly supported on nearly all hardware, so
users do not use it.

Since we have to change from DXC anyway, we are using VK1.3. It has been
out long enough to be commonly available, and the initial implementation
of SPIR-V features for HLSL are assuming Vulkan 1.3.

---------

Co-authored-by: Nathan Gauër <github@keenuts.net>
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
The HLSL driver currently defaults the triple to an unversioned os and
subarch when targeting SPIR-V. This means the SPIR-V backend decides the
default value. That is not a great option because a change the backend
could cause a change in Clang.

Now that we want to choose the default we need to consider the best
option. DXC currently defaults to Vulkan1.0. We are planning on not
supporting Vulkan1.0 in the Clang HLSL compiler because it is newer
versions of Vulkan are commonly supported on nearly all hardware, so
users do not use it.

Since we have to change from DXC anyway, we are using VK1.3. It has been
out long enough to be commonly available, and the initial implementation
of SPIR-V features for HLSL are assuming Vulkan 1.3.

---------

Co-authored-by: Nathan Gauër <github@keenuts.net>
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 Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants