From be7e42b7e8953278a84a98dd05c8e8a31a91df1d Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Thu, 10 Jul 2025 14:48:06 -0700 Subject: [PATCH 1/3] [libclc][test] Really run libspirv tests on all targets Due to the way `add_lit_testsuite` works, the `check-all` target was not running the libspirv (binding) tests on all targets, instead it was running it on the last target multiple times in parallel. Besides losing the coverage for other targets, this also caused sporadic failures due to tests overwriting each other. https://github.com/intel/llvm/pull/17902 was an attempt to fix this, and it works for running all of the `check-libclc-spirv-*` cmake targets in parallel, but it does not work for the `check-all` target, which is what we use in the CI. This patch fixes the issue by making a different `lit` test suite for each target. See the comment in `libclc/test/CMakeLists.txt` for more details. --- libclc/test/CMakeLists.txt | 50 +++++++++++++++++----------------- libclc/test/lit.cfg.py | 32 ++++++++++++---------- libclc/test/lit.site.cfg.py.in | 14 +++++++++- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/libclc/test/CMakeLists.txt b/libclc/test/CMakeLists.txt index e7ab8713ce2e..d14581c561e3 100644 --- a/libclc/test/CMakeLists.txt +++ b/libclc/test/CMakeLists.txt @@ -1,5 +1,6 @@ # required by lit.site.cfg.py.in set(LIBCLC_TEST_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) +set(LIBCLC_PERTARGET_TEST_DIR ${CMAKE_CURRENT_BINARY_DIR}/targets) configure_lit_site_cfg( ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in @@ -16,40 +17,39 @@ set(LIBCLC_TEST_DEPS count ) -add_custom_target(check-libclc) - +umbrella_lit_testsuite_begin(check-libclc) foreach( t ${LIBCLC_TARGET_TO_TEST} ) - foreach( d ${${t}_devices} ) - if( ${d} STREQUAL "none" ) - set( mcpu ) - set( arch_suffix "${t}" ) - else() - set( mcpu "cpu=${d}" ) - set( arch_suffix "${d}-${t}" ) - endif() - message( " Testing : ${arch_suffix}" ) - - add_lit_testsuite(check-libclc-spirv-${arch_suffix} - "Running libclc spirv-${arch_suffix} regression tests" - ${CMAKE_CURRENT_BINARY_DIR} - ARGS - --verbose - PARAMS "target=${t}" ${mcpu} "libclc-arch-suffix=${arch_suffix}" - "builtins=libspirv-${arch_suffix}.bc" + # Each target gets own lit testsuite directory. + # This is done because all suites passed to add_lit_testsuite + # are collected into a single invocation of lit, for example: + # add_lit_testsuite(check-a dir PARAMS "foo=1") + # add_lit_testsuite(check-b dir PARAMS "foo=2" "bar=3") + # results in a call to lit like this (for `check-all`): + # llvm-lit --param=foo=1 --param=foo=2 --param=bar=3 dir dir + # This means we can't reuse the same directory and rely on + # different parameter values to distinguish between targets, because + # parameters with the same name overwrite each other. + # The solution is to create a separate directory for each target, + # and copy the lit.site.cfg.py file there; the name of the + # directory will be used as the target name. + file(COPY_FILE ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py + ${LIBCLC_PERTARGET_TEST_DIR}/${t}/lit.site.cfg.py) + + add_lit_testsuite(check-libclc-spirv-${t} + "Running libclc spirv-${t} regression tests" + ${LIBCLC_PERTARGET_TEST_DIR}/${t} DEPENDS ${LIBCLC_TEST_DEPS} libspirv-builtins ) - - add_dependencies(check-libclc check-libclc-spirv-${arch_suffix}) - - endforeach( d ) endforeach( t ) if(LIBCLC_GENERATE_REMANGLED_VARIANTS) - #Now that check-libclc is defined make sure that all remangler targets depend + # Now that check-libclc is defined make sure that all remangler targets depend # on it. foreach(remangler-test ${libclc-remangler-tests}) - add_dependencies(check-libclc ${remangler-test}) + set_property(GLOBAL APPEND PROPERTY LLVM_LIBCLC_ADDITIONAL_TEST_TARGETS ${remangler-test}) + set_property(GLOBAL APPEND PROPERTY LLVM_ALL_ADDITIONAL_TEST_TARGETS ${remangler-test}) endforeach() endif() +umbrella_lit_testsuite_end(check-libclc) diff --git a/libclc/test/lit.cfg.py b/libclc/test/lit.cfg.py index d56204d569d6..142b11dd0fa8 100644 --- a/libclc/test/lit.cfg.py +++ b/libclc/test/lit.cfg.py @@ -11,8 +11,20 @@ # Configuration file for the 'lit' test runner. +if config.libclc_target is None: + lit_config.fatal("libclc_target parameter must be set when running directly") + +def quote(s): + return f"'{s}'" + +if config.libclc_target not in config.libclc_targets_to_test: + lit_config.fatal( + f"libclc_target '{config.libclc_target}' is not built. " + f"Available targets: {', '.join(quote(s) for s in config.libclc_targets_to_test)}") + # name: The name of this test suite. -config.name = "LIBCLC" +config.name = f"LIBCLC-{config.libclc_target.upper()}" + # testFormat: The test format to use to interpret tests. config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell) @@ -28,16 +40,10 @@ libclc_inc = os.path.join(config.libclc_root, "libspirv", "include") -target = lit_config.params.get("target", "") -builtins = lit_config.params.get("builtins", "") -cpu = lit_config.params.get("cpu", "") -cpu = [] if cpu == "" else ["-mcpu=" + cpu] -libclc_arch_suffix = lit_config.params.get("libclc-arch-suffix", "") - # test_exec_root: The root path where tests should be run. We create a unique # test directory per libclc target to test to avoid data races when multiple # targets try and access the the same libclc test files. -config.test_exec_root = os.path.join(config.test_run_dir, "test", libclc_arch_suffix) +config.test_exec_root = os.path.join(config.libclc_pertarget_test_dir, config.libclc_target) llvm_config.use_default_substitutions() @@ -46,24 +52,22 @@ "-I", libclc_inc, "-target", - target, + config.libclc_target, "-Xclang", "-fdeclare-spirv-builtins", "-Xclang", "-mlink-builtin-bitcode", "-Xclang", - os.path.join(config.llvm_libs_dir, "clc", builtins), + os.path.join(config.llvm_libs_dir, "clc", f"libspirv-{config.libclc_target}.bc"), "-nogpulib", ] -if target == "amdgcn--amdhsa": - config.available_features.add("amdgcn") - +if config.libclc_target == "amdgcn--amdhsa": # libclc for amdgcn is currently built for tahiti which doesn't support # fp16 so disable the extension for the tests clang_flags += ["-Xclang", "-cl-ext=-cl_khr_fp16"] -llvm_config.use_clang(additional_flags=clang_flags + cpu) +llvm_config.use_clang(additional_flags=clang_flags) config.substitutions.append(("%PATH%", config.environment["PATH"])) diff --git a/libclc/test/lit.site.cfg.py.in b/libclc/test/lit.site.cfg.py.in index e4ff02a682c1..b6614c8e62f8 100644 --- a/libclc/test/lit.site.cfg.py.in +++ b/libclc/test/lit.site.cfg.py.in @@ -1,5 +1,6 @@ @LIT_SITE_CFG_IN_HEADER@ +import os import sys config.llvm_src_root = "@LLVM_SOURCE_DIR@" @@ -14,10 +15,21 @@ config.target_triple = "@LLVM_TARGET_TRIPLE@" config.host_arch = "@HOST_ARCH@" config.python_executable = "@PYTHON_EXECUTABLE@" config.libclc_root = "@LIBCLC_SOURCE_DIR@" -config.test_run_dir = "@LIBCLC_BINARY_DIR@" +config.libclc_binary_dir = "@LIBCLC_BINARY_DIR@" +config.libclc_targets_to_test = "@LIBCLC_TARGET_TO_TEST@".split(";") +config.libclc_pertarget_test_dir = "@LIBCLC_PERTARGET_TEST_DIR@" import lit.llvm lit.llvm.initialize(lit_config, config) +config.libclc_target = lit_config.params.get("libclc_target") + +if config.libclc_target is None: + # This file is copied to per-target test directories by cmake, + # use the name of the directory containing this file as the target name. + dirname = os.path.dirname(__file__) + if os.path.realpath(dirname) != os.path.realpath("@CMAKE_CURRENT_BINARY_DIR@"): + config.libclc_target = os.path.basename(dirname) + # Let the main config do the real work. lit_config.load_config(config, "@LIBCLC_TEST_SOURCE_DIR@/lit.cfg.py") From fc808fcb800b5b666dba06523badd189229d04f5 Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Thu, 10 Jul 2025 15:15:58 -0700 Subject: [PATCH 2/3] mkdir, fix formatting --- libclc/test/CMakeLists.txt | 2 ++ libclc/test/lit.cfg.py | 15 ++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/libclc/test/CMakeLists.txt b/libclc/test/CMakeLists.txt index d14581c561e3..b846cf7c5b77 100644 --- a/libclc/test/CMakeLists.txt +++ b/libclc/test/CMakeLists.txt @@ -2,6 +2,8 @@ set(LIBCLC_TEST_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(LIBCLC_PERTARGET_TEST_DIR ${CMAKE_CURRENT_BINARY_DIR}/targets) +file(MAKE_DIRECTORY ${LIBCLC_PERTARGET_TEST_DIR}) + configure_lit_site_cfg( ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py diff --git a/libclc/test/lit.cfg.py b/libclc/test/lit.cfg.py index 142b11dd0fa8..048565af0dc3 100644 --- a/libclc/test/lit.cfg.py +++ b/libclc/test/lit.cfg.py @@ -9,18 +9,21 @@ from lit.llvm.subst import ToolSubst from lit.llvm.subst import FindTool + +def quote(s): + return f"'{s}'" + + # Configuration file for the 'lit' test runner. if config.libclc_target is None: lit_config.fatal("libclc_target parameter must be set when running directly") -def quote(s): - return f"'{s}'" - if config.libclc_target not in config.libclc_targets_to_test: lit_config.fatal( f"libclc_target '{config.libclc_target}' is not built. " - f"Available targets: {', '.join(quote(s) for s in config.libclc_targets_to_test)}") + f"Available targets: {', '.join(quote(s) for s in config.libclc_targets_to_test)}" + ) # name: The name of this test suite. config.name = f"LIBCLC-{config.libclc_target.upper()}" @@ -43,7 +46,9 @@ def quote(s): # test_exec_root: The root path where tests should be run. We create a unique # test directory per libclc target to test to avoid data races when multiple # targets try and access the the same libclc test files. -config.test_exec_root = os.path.join(config.libclc_pertarget_test_dir, config.libclc_target) +config.test_exec_root = os.path.join( + config.libclc_pertarget_test_dir, config.libclc_target +) llvm_config.use_default_substitutions() From 5a72bb2264ef5334bcf8bb222800700919f9f6f5 Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Fri, 11 Jul 2025 00:39:10 -0700 Subject: [PATCH 3/3] mkdir, properly this time --- libclc/test/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libclc/test/CMakeLists.txt b/libclc/test/CMakeLists.txt index b846cf7c5b77..8232f7bc17af 100644 --- a/libclc/test/CMakeLists.txt +++ b/libclc/test/CMakeLists.txt @@ -2,8 +2,6 @@ set(LIBCLC_TEST_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(LIBCLC_PERTARGET_TEST_DIR ${CMAKE_CURRENT_BINARY_DIR}/targets) -file(MAKE_DIRECTORY ${LIBCLC_PERTARGET_TEST_DIR}) - configure_lit_site_cfg( ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py @@ -34,6 +32,7 @@ foreach( t ${LIBCLC_TARGET_TO_TEST} ) # The solution is to create a separate directory for each target, # and copy the lit.site.cfg.py file there; the name of the # directory will be used as the target name. + file(MAKE_DIRECTORY ${LIBCLC_PERTARGET_TEST_DIR}/${t}) file(COPY_FILE ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py ${LIBCLC_PERTARGET_TEST_DIR}/${t}/lit.site.cfg.py)