-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[clang-linker-wrapper][lit] Fix SPIR-V ELF test when spirv-tools feature is available #126756
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
Conversation
…ure is available Signed-off-by: Sarnie, Nick <[email protected]>
Signed-off-by: Sarnie, Nick <[email protected]>
@llvm/pr-subscribers-clang Author: Nick Sarnie (sarnex) ChangesMy last change made the test not run when the
Manually confirmed this works. This test is the bane of my existence. Full diff: https://github.com/llvm/llvm-project/pull/126756.diff 3 Files Affected:
diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index e9eb54a67204c..b796a51ef600e 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -103,6 +103,15 @@ if(CLANG_BUILD_EXAMPLES AND CLANG_PLUGIN_SUPPORT)
)
endif ()
+if(LLVM_INCLUDE_SPIRV_TOOLS_TESTS)
+ list(APPEND CLANG_TEST_DEPS
+ spirv-dis
+ spirv-val
+ spirv-as
+ spirv-link
+ )
+endif()
+
set(CLANG_TEST_PARAMS
USE_Z3_SOLVER=0
)
diff --git a/clang/test/Tooling/clang-linker-wrapper-spirv-elf.cpp b/clang/test/Tooling/clang-linker-wrapper-spirv-elf.cpp
index 4f8658064e857..9b16727d74192 100644
--- a/clang/test/Tooling/clang-linker-wrapper-spirv-elf.cpp
+++ b/clang/test/Tooling/clang-linker-wrapper-spirv-elf.cpp
@@ -1,6 +1,7 @@
// Verify the ELF packaging of OpenMP SPIR-V device images.
// REQUIRES: system-linux
// REQUIRES: spirv-tools
+// REQUIRES: llvm-spirv
// RUN: mkdir -p %t_tmp
// RUN: cd %t_tmp
// RUN: %clangxx -fopenmp -fopenmp-targets=spirv64-intel -nogpulib -c -o %t_clang-linker-wrapper-spirv-elf.o %s
diff --git a/clang/test/Tooling/lit.local.cfg b/clang/test/Tooling/lit.local.cfg
index bc2a096c8f64f..61f328c91e4d3 100644
--- a/clang/test/Tooling/lit.local.cfg
+++ b/clang/test/Tooling/lit.local.cfg
@@ -1,3 +1,5 @@
+import lit.util
+
if not config.root.clang_staticanalyzer:
config.unsupported = True
@@ -6,3 +8,7 @@ if config.spirv_tools_tests:
config.substitutions.append(("spirv-dis", os.path.join(config.llvm_tools_dir, "spirv-dis")))
config.substitutions.append(("spirv-val", os.path.join(config.llvm_tools_dir, "spirv-val")))
config.substitutions.append(("spirv-as", os.path.join(config.llvm_tools_dir, "spirv-as")))
+ config.substitutions.append(("spirv-link", os.path.join(config.llvm_tools_dir, "spirv-link")))
+
+if lit.util.which("llvm-spirv"):
+ config.available_features.add("llvm-spirv")
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've all been there
…ure is available (llvm#126756) My last change made the test not run when the `spirv-tools` feature is not available, which is always the case in CI for clang tests, but it fails if `spirv-tools` is available for the following reasons: 1) We didn't build `spirv-link` as part of the internal `SPIRV-Tools` build, which is required by the `clang` call in `clang-linker-wrapper`, I already fixed that [here](llvm#126319). 2) We didn't depend on the `SPIRV-Tools` CMake targets in clang tests, so depending on what CMake targets were built before running `check-clang`, `SPIRV-Tools` might not have been built. 3) We didn't check for `llvm-spirv` being available, which is not part of `SPIRV-Tools` but is currently required for SPIR-V compilation. Manually confirmed this works. This test is the bane of my existence. --------- Signed-off-by: Sarnie, Nick <[email protected]>
…ure is available (llvm#126756) My last change made the test not run when the `spirv-tools` feature is not available, which is always the case in CI for clang tests, but it fails if `spirv-tools` is available for the following reasons: 1) We didn't build `spirv-link` as part of the internal `SPIRV-Tools` build, which is required by the `clang` call in `clang-linker-wrapper`, I already fixed that [here](llvm#126319). 2) We didn't depend on the `SPIRV-Tools` CMake targets in clang tests, so depending on what CMake targets were built before running `check-clang`, `SPIRV-Tools` might not have been built. 3) We didn't check for `llvm-spirv` being available, which is not part of `SPIRV-Tools` but is currently required for SPIR-V compilation. Manually confirmed this works. This test is the bane of my existence. --------- Signed-off-by: Sarnie, Nick <[email protected]>
…ure is available (llvm#126756) My last change made the test not run when the `spirv-tools` feature is not available, which is always the case in CI for clang tests, but it fails if `spirv-tools` is available for the following reasons: 1) We didn't build `spirv-link` as part of the internal `SPIRV-Tools` build, which is required by the `clang` call in `clang-linker-wrapper`, I already fixed that [here](llvm#126319). 2) We didn't depend on the `SPIRV-Tools` CMake targets in clang tests, so depending on what CMake targets were built before running `check-clang`, `SPIRV-Tools` might not have been built. 3) We didn't check for `llvm-spirv` being available, which is not part of `SPIRV-Tools` but is currently required for SPIR-V compilation. Manually confirmed this works. This test is the bane of my existence. --------- Signed-off-by: Sarnie, Nick <[email protected]>
…ure is available (llvm#126756) My last change made the test not run when the `spirv-tools` feature is not available, which is always the case in CI for clang tests, but it fails if `spirv-tools` is available for the following reasons: 1) We didn't build `spirv-link` as part of the internal `SPIRV-Tools` build, which is required by the `clang` call in `clang-linker-wrapper`, I already fixed that [here](llvm#126319). 2) We didn't depend on the `SPIRV-Tools` CMake targets in clang tests, so depending on what CMake targets were built before running `check-clang`, `SPIRV-Tools` might not have been built. 3) We didn't check for `llvm-spirv` being available, which is not part of `SPIRV-Tools` but is currently required for SPIR-V compilation. Manually confirmed this works. This test is the bane of my existence. --------- Signed-off-by: Sarnie, Nick <[email protected]>
My last change made the test not run when the
spirv-tools
feature is not available, which is always the case in CI for clang tests, but it fails ifspirv-tools
is available for the following reasons:spirv-link
as part of the internalSPIRV-Tools
build, which is required by theclang
call inclang-linker-wrapper
, I already fixed that here.SPIRV-Tools
CMake targets in clang tests, so depending on what CMake targets were built before runningcheck-clang
,SPIRV-Tools
might not have been built.llvm-spirv
being available, which is not part ofSPIRV-Tools
but is currently required for SPIR-V compilation.Manually confirmed this works. This test is the bane of my existence.