[libclc][test] Really run libspirv tests on all targets (#19395)

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.
#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.

Author: Maetveis

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,40 @@ 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(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)
+
+    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)

libclc/test/lit.cfg.py

@@ -9,10 +9,25 @@
 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")
+
+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 +43,12 @@
 
 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 +57,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"]))
 

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")