Skip to content

[clang][Driver][SPIR-V] Make tool names consistent #122343

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

Merged
merged 1 commit into from
Jan 10, 2025
Merged

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 bader, svenvh and MrSidims 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-fix-vnni-instr-types 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