Skip to content

Conversation

@s-perron
Copy link
Contributor

@s-perron s-perron commented Jan 7, 2025

In DXC, setting the vulkan version automatically sets the target spir-v
version to the maximum spir-v version that the vulkan version must
support. So for Vulkan 1.2, we set the spir-v version to spirv 1.5
because every implementation of Vulkan 1.2 must support spirv 1.5, but
not spir-v 1.6.

In DXC, setting the vulkan version automatically sets the target spir-v
version to the maximum spir-v version that the vulkan version must
support. So for Vulkan 1.2, we set the spir-v version to spirv 1.5
because every implementation of Vulkan 1.2 must support spirv 1.5, but
not spir-v 1.6.
@s-perron s-perron requested a review from Keenuts January 7, 2025 16:28
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jan 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-clang-driver

Author: Steven Perron (s-perron)

Changes

In DXC, setting the vulkan version automatically sets the target spir-v
version to the maximum spir-v version that the vulkan version must
support. So for Vulkan 1.2, we set the spir-v version to spirv 1.5
because every implementation of Vulkan 1.2 must support spirv 1.5, but
not spir-v 1.6.


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

3 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+8-3)
  • (modified) clang/test/Driver/dxc_spirv.hlsl (+2-2)
  • (modified) llvm/lib/TargetParser/Triple.cpp (+20)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 36d6c93c43321f..a31fb3fc08a42f 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1468,9 +1468,14 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
 
         // Set specific Vulkan version if applicable.
         if (const Arg *A = Args.getLastArg(options::OPT_fspv_target_env_EQ)) {
-          const llvm::StringSet<> ValidValues = {"vulkan1.2", "vulkan1.3"};
-          if (ValidValues.contains(A->getValue())) {
-            T.setOSName(A->getValue());
+          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 {
             Diag(diag::err_drv_invalid_value)
                 << A->getAsString(Args) << A->getValue();
diff --git a/clang/test/Driver/dxc_spirv.hlsl b/clang/test/Driver/dxc_spirv.hlsl
index c087ea4b0d709f..e6624e5f1b3f6f 100644
--- a/clang/test/Driver/dxc_spirv.hlsl
+++ b/clang/test/Driver/dxc_spirv.hlsl
@@ -6,8 +6,8 @@
 // CHECK: "-triple" "spirv-unknown-vulkan-compute"
 // CHECK-SAME: "-x" "hlsl"
 
-// CHECK-VULKAN12: "-triple" "spirv-unknown-vulkan1.2-compute"
+// CHECK-VULKAN12: "-triple" "spirv1.5-unknown-vulkan1.2-compute"
 
-// CHECK-VULKAN13: "-triple" "spirv-unknown-vulkan1.3-compute"
+// CHECK-VULKAN13: "-triple" "spirv1.6-unknown-vulkan1.3-compute"
 
 // CHECK-ERROR: error: invalid value 'vulkan1.0' in '-fspv-target-env=vulkan1.0'
diff --git a/llvm/lib/TargetParser/Triple.cpp b/llvm/lib/TargetParser/Triple.cpp
index 7e040688dc1a7b..4c1de09e91f21c 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -113,6 +113,26 @@ StringRef Triple::getArchName(ArchType Kind, SubArchType SubArch) {
     if (SubArch == AArch64SubArch_arm64e)
       return "arm64e";
     break;
+  case Triple::spirv:
+    switch (SubArch) {
+    case Triple::SPIRVSubArch_v10:
+      return "spirv1.0";
+    case Triple::SPIRVSubArch_v11:
+      return "spirv1.1";
+    case Triple::SPIRVSubArch_v12:
+      return "spirv1.2";
+    case Triple::SPIRVSubArch_v13:
+      return "spirv1.3";
+    case Triple::SPIRVSubArch_v14:
+      return "spirv1.4";
+    case Triple::SPIRVSubArch_v15:
+      return "spirv1.5";
+    case Triple::SPIRVSubArch_v16:
+      return "spirv1.6";
+    default:
+      break;
+    }
+    break;
   case Triple::dxil:
     switch (SubArch) {
     case Triple::NoSubArch:

@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-clang

Author: Steven Perron (s-perron)

Changes

In DXC, setting the vulkan version automatically sets the target spir-v
version to the maximum spir-v version that the vulkan version must
support. So for Vulkan 1.2, we set the spir-v version to spirv 1.5
because every implementation of Vulkan 1.2 must support spirv 1.5, but
not spir-v 1.6.


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

3 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+8-3)
  • (modified) clang/test/Driver/dxc_spirv.hlsl (+2-2)
  • (modified) llvm/lib/TargetParser/Triple.cpp (+20)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 36d6c93c43321f..a31fb3fc08a42f 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1468,9 +1468,14 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
 
         // Set specific Vulkan version if applicable.
         if (const Arg *A = Args.getLastArg(options::OPT_fspv_target_env_EQ)) {
-          const llvm::StringSet<> ValidValues = {"vulkan1.2", "vulkan1.3"};
-          if (ValidValues.contains(A->getValue())) {
-            T.setOSName(A->getValue());
+          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 {
             Diag(diag::err_drv_invalid_value)
                 << A->getAsString(Args) << A->getValue();
diff --git a/clang/test/Driver/dxc_spirv.hlsl b/clang/test/Driver/dxc_spirv.hlsl
index c087ea4b0d709f..e6624e5f1b3f6f 100644
--- a/clang/test/Driver/dxc_spirv.hlsl
+++ b/clang/test/Driver/dxc_spirv.hlsl
@@ -6,8 +6,8 @@
 // CHECK: "-triple" "spirv-unknown-vulkan-compute"
 // CHECK-SAME: "-x" "hlsl"
 
-// CHECK-VULKAN12: "-triple" "spirv-unknown-vulkan1.2-compute"
+// CHECK-VULKAN12: "-triple" "spirv1.5-unknown-vulkan1.2-compute"
 
-// CHECK-VULKAN13: "-triple" "spirv-unknown-vulkan1.3-compute"
+// CHECK-VULKAN13: "-triple" "spirv1.6-unknown-vulkan1.3-compute"
 
 // CHECK-ERROR: error: invalid value 'vulkan1.0' in '-fspv-target-env=vulkan1.0'
diff --git a/llvm/lib/TargetParser/Triple.cpp b/llvm/lib/TargetParser/Triple.cpp
index 7e040688dc1a7b..4c1de09e91f21c 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -113,6 +113,26 @@ StringRef Triple::getArchName(ArchType Kind, SubArchType SubArch) {
     if (SubArch == AArch64SubArch_arm64e)
       return "arm64e";
     break;
+  case Triple::spirv:
+    switch (SubArch) {
+    case Triple::SPIRVSubArch_v10:
+      return "spirv1.0";
+    case Triple::SPIRVSubArch_v11:
+      return "spirv1.1";
+    case Triple::SPIRVSubArch_v12:
+      return "spirv1.2";
+    case Triple::SPIRVSubArch_v13:
+      return "spirv1.3";
+    case Triple::SPIRVSubArch_v14:
+      return "spirv1.4";
+    case Triple::SPIRVSubArch_v15:
+      return "spirv1.5";
+    case Triple::SPIRVSubArch_v16:
+      return "spirv1.6";
+    default:
+      break;
+    }
+    break;
   case Triple::dxil:
     switch (SubArch) {
     case Triple::NoSubArch:

@s-perron
Copy link
Contributor Author

s-perron commented Jan 8, 2025

Failure is unrelated to my change.

@s-perron s-perron requested a review from Keenuts January 8, 2025 15:18
@s-perron s-perron merged commit d723587 into llvm:main Jan 9, 2025
9 of 11 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 9, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building clang,llvm at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/11439

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/test_libc.cpp (986 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49779.cpp (987 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug50022.cpp (988 of 999)
PASS: libomptarget :: amdgcn-amd-amdhsa :: offloading/ompx_saxpy_mixed.c (989 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/wtime.c (990 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp (991 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp (992 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/complex_reduction.cpp (993 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp (994 of 999)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (995 of 999)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1236.512420

@s-perron s-perron deleted the spirv_version branch January 13, 2025 16:39
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.

4 participants