Skip to content

Conversation

@sarnex
Copy link
Member

@sarnex sarnex commented Jan 9, 2025

Some use SPIRV and some use SPIR-V, just use SPIR-V which is what we use normally.

I noticed this when fixing the test in #122310.

@sarnex sarnex marked this pull request as ready for review January 10, 2025 15:38
@sarnex sarnex requested review from MrSidims, bader and svenvh January 10, 2025 15:39
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' backend:SPIR-V labels Jan 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 10, 2025

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-backend-spir-v

@llvm/pr-subscribers-clang

Author: Nick Sarnie (sarnex)

Changes

Some use SPIRV and some use SPIR-V, just use SPIR-V which is what we use normally.

I noticed this when fixing the test in #122310.


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

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/SPIRV.h (+2-2)
  • (modified) clang/test/Driver/spirv-openmp-toolchain.c (+1-1)
  • (modified) clang/test/Driver/spirv-toolchain.cl (+10)
diff --git a/clang/lib/Driver/ToolChains/SPIRV.h b/clang/lib/Driver/ToolChains/SPIRV.h
index 44187084e34ec4..6223d55eca6435 100644
--- a/clang/lib/Driver/ToolChains/SPIRV.h
+++ b/clang/lib/Driver/ToolChains/SPIRV.h
@@ -43,7 +43,7 @@ class LLVM_LIBRARY_VISIBILITY Translator : public Tool {
 
 class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
 public:
-  Linker(const ToolChain &TC) : Tool("SPIRV::Linker", "spirv-link", TC) {}
+  Linker(const ToolChain &TC) : Tool("SPIR-V::Linker", "spirv-link", TC) {}
   bool hasIntegratedCPP() const override { return false; }
   bool isLinkJob() const override { return true; }
   void ConstructJob(Compilation &C, const JobAction &JA,
@@ -54,7 +54,7 @@ class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
 
 class LLVM_LIBRARY_VISIBILITY Assembler final : public Tool {
 public:
-  Assembler(const ToolChain &TC) : Tool("SPIRV::Assembler", "spirv-as", TC) {}
+  Assembler(const ToolChain &TC) : Tool("SPIR-V::Assembler", "spirv-as", TC) {}
   bool hasIntegratedAssembler() const override { return false; }
   bool hasIntegratedCPP() const override { return false; }
   void ConstructJob(Compilation &C, const JobAction &JA,
diff --git a/clang/test/Driver/spirv-openmp-toolchain.c b/clang/test/Driver/spirv-openmp-toolchain.c
index 3d585d78e86c21..5a76bf70e7ed35 100644
--- a/clang/test/Driver/spirv-openmp-toolchain.c
+++ b/clang/test/Driver/spirv-openmp-toolchain.c
@@ -45,7 +45,7 @@
 // CHECK-BINDINGS-TEMPS: "spirv64-intel" - "clang", inputs: ["[[INPUT]]"], output: "[[DEVICE_PP:.+]]"
 // CHECK-BINDINGS-TEMPS: "spirv64-intel" - "clang", inputs: ["[[DEVICE_PP]]", "[[HOST_BC]]"], output: "[[DEVICE_TEMP_BC:.+]]"
 // CHECK-BINDINGS-TEMPS: "spirv64-intel" - "SPIR-V::Translator", inputs: ["[[DEVICE_TEMP_BC]]"], output: "[[DEVICE_ASM:.+]]"
-// CHECK-BINDINGS-TEMPS: "spirv64-intel" - "SPIRV::Assembler", inputs: ["[[DEVICE_ASM]]"], output: "[[DEVICE_SPV:.+]]"
+// CHECK-BINDINGS-TEMPS: "spirv64-intel" - "SPIR-V::Assembler", inputs: ["[[DEVICE_ASM]]"], output: "[[DEVICE_SPV:.+]]"
 // CHECK-BINDINGS-TEMPS: "x86_64-unknown-linux-gnu" - "Offload::Packager", inputs: ["[[DEVICE_SPV]]"], output: "[[DEVICE_IMAGE:.+]]"
 // CHECK-BINDINGS-TEMPS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_IMAGE]]"], output: "[[HOST_ASM:.+]]"
 // CHECK-BINDINGS-TEMPS: "x86_64-unknown-linux-gnu" - "clang::as", inputs: ["[[HOST_ASM]]"], output: "[[HOST_OBJ:.+]]"
diff --git a/clang/test/Driver/spirv-toolchain.cl b/clang/test/Driver/spirv-toolchain.cl
index 33c7bc0a63adfc..fd29dbe71e9105 100644
--- a/clang/test/Driver/spirv-toolchain.cl
+++ b/clang/test/Driver/spirv-toolchain.cl
@@ -70,6 +70,16 @@
 // SPLINK: {{llvm-spirv.*"}} [[BC]] "-o" [[SPV2:".*o"]]
 // SPLINK: {{spirv-link.*"}} [[SPV1]] [[SPV2]] "-o" "a.out"
 
+//-----------------------------------------------------------------------------
+// Check bindings when linking when multiple input files are passed.
+// RUN: %clang -### -target spirv64 -ccc-print-bindings %s %s 2>&1 | FileCheck --check-prefix=SPLINK-BINDINGS %s
+
+// SPLINK-BINDINGS: "clang", inputs: [[[CL:".*cl"]]], output: [[BC1:".*bc"]]
+// SPLINK-BINDINGS: "SPIR-V::Translator", inputs: [[[BC1]]], output: [[OBJ1:".*o"]]
+// SPLINK-BINDINGS: "clang", inputs: [[[CL]]], output: [[BC2:".*bc"]]
+// SPLINK-BINDINGS: "SPIR-V::Translator", inputs: [[[BC2]]], output: [[OBJ2:".*o"]]
+// SPLINK-BINDINGS: "SPIR-V::Linker", inputs: [[[OBJ1]], [[OBJ2]]], output: "a.out"
+
 //-----------------------------------------------------------------------------
 // Check external vs internal object emission.
 // RUN: %clang -### --target=spirv64 -fno-integrated-objemitter %s 2>&1 | FileCheck --check-prefix=XTOR %s

Copy link
Member

@svenvh svenvh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks; good to see consistent naming!

@sarnex sarnex merged commit c391082 into llvm:main Jan 10, 2025
14 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 10, 2025

LLVM Buildbot has detected a new failure on builder arc-builder running on arc-worker while building clang at step 6 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lit :: googletest-timeout.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 9
not env -u FILECHECK_OPTS "/usr/local/bin/python3.9" /buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout    --param gtest_filter=InfiniteLoopSubTest --timeout=1 > /buildbot/worker/arc-folder/build/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out
# executed command: not env -u FILECHECK_OPTS /usr/local/bin/python3.9 /buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --timeout=1
# .---command stderr------------
# | lit.py: /buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# | Traceback (most recent call last):
# |   File "/buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 304, in post_process_shard_results
# |     testsuites = json.load(f)["testsuites"]
# |   File "/usr/local/lib/python3.9/json/__init__.py", line 293, in load
# |     return loads(fp.read(),
# |   File "/usr/local/lib/python3.9/json/__init__.py", line 346, in loads
# |     return _default_decoder.decode(s)
# |   File "/usr/local/lib/python3.9/json/decoder.py", line 337, in decode
# |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
# |   File "/usr/local/lib/python3.9/json/decoder.py", line 355, in raw_decode
# |     raise JSONDecodeError("Expecting value", s, err.value) from None
# | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
# | 
# | During handling of the above exception, another exception occurred:
# | 
# | Traceback (most recent call last):
# |   File "/buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit.py", line 6, in <module>
# |     main()
# |   File "/buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit/main.py", line 130, in main
# |     selected_tests, discovered_tests = GoogleTest.post_process_shard_results(
# |   File "/buildbot/worker/arc-folder/llvm-project/llvm/utils/lit/lit/formats/googletest.py", line 306, in post_process_shard_results
# |     raise RuntimeError(
# | RuntimeError: Failed to parse json file: /buildbot/worker/arc-folder/build/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py-googletest-timeout-24166-1-2.json
# | 
# `-----------------------------
# RUN: at line 11
FileCheck --check-prefix=CHECK-INF < /buildbot/worker/arc-folder/build/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out /buildbot/worker/arc-folder/build/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /buildbot/worker/arc-folder/build/utils/lit/tests/googletest-timeout.py
# .---command stderr------------
# | /buildbot/worker/arc-folder/build/utils/lit/tests/googletest-timeout.py:34:14: error: CHECK-INF: expected string not found in input
# | # CHECK-INF: Timed Out: 1
# |              ^
# | <stdin>:13:29: note: scanning from here
# | Reached timeout of 1 seconds
# |                             ^
# | <stdin>:15:21: note: possible intended match here
# | TIMEOUT: googletest-timeout :: DummySubDir/OneTest.py/1/2 (2 of 2)
# |                     ^
# | 
# | Input file: <stdin>
...

BaiXilin pushed a commit to BaiXilin/llvm-project that referenced this pull request Jan 12, 2025
Some use `SPIRV` and some use `SPIR-V`, just use `SPIR-V` which is what
we use normally.

I noticed this when fixing the test in
llvm#122310.

Signed-off-by: Sarnie, Nick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:SPIR-V 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.

5 participants