From 72009274bd77f5d7cf7fec4f6e4cc3d4d09dabf8 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Aug 2026 07:09:40 -0700 Subject: [PATCH 1/2] Check the dispatch-surface declaration, and keep it configurable The declaration added in the preceding commit is only worth something if it is checked rather than trusted, and if the knob that overrides it is actually turned by something other than a person debugging. Move the validation out of `generate-dispatch-surface.cmake` into `validate-dispatch-surface.cmake`, which touches no build-system state and so runs in script mode: cmake -DSVS_DISPATCH_SURFACE_FILE= -DSVS_X86_SRC_DIR= \ -P cmake/validate-dispatch-surface.cmake `tests/cmake/dispatch-surface/` holds two declarations that must be accepted and eleven that must be rejected, each carrying the substring its rejection has to mention. `.github/scripts/check_dispatch_surface.sh` runs the lot -- plus the default declaration -- in 0.2s, needing nothing but cmake. It is a pre-commit hook and a CI job. Script mode has no `cmake_minimum_required`, so CMP0007 and CMP0057 default to OLD there. Both matter: without CMP0007 an empty `|`-field disappears when the entry is split, and without CMP0057 `IN_LIST` is not an operator. Set both, scoped with cmake_policy PUSH/POP. The new `Dispatch Surface` workflow adds what the script cannot check: - a configure with the default declaration must leave the committed `dispatch_surface.h` untouched. This catches a declaration changed without a reconfigure, and a generated header edited by hand. - a full build and test run against `valid-reduced.cmake`, which shares no extent with the default declaration -- so a build that quietly fell back to the committed header would fail to compile rather than pass by accident. That build's archive holds 288 kernels at extents 32, 384 and svs::Dynamic, against 864 at the default nine. - that same overridden build must leave the committed header alone. Correctness does not depend on which extents have a fixed-extent kernel: an extent without one is served by the svs::Dynamic kernel. `ctest -LE long` against the reduced surface passes 153 of 154, the one failure being `Testing Binary Reader Iterator`, which fails identically on the unmodified default-surface build. Co-Authored-By: Claude Opus 5 --- .github/scripts/check_dispatch_surface.sh | 111 +++++++++++++ .github/workflows/dispatch-surface.yml | 106 +++++++++++++ .pre-commit-config.yaml | 11 ++ cmake/generate-dispatch-surface.cmake | 116 ++------------ cmake/validate-dispatch-surface.cmake | 148 ++++++++++++++++++ .../invalid-duplicate-extent.cmake | 17 ++ .../invalid-duplicate-infix.cmake | 22 +++ .../invalid-duplicate-level.cmake | 21 +++ .../invalid-dynamic-listed.cmake | 18 +++ .../invalid-level-empty-field.cmake | 17 ++ .../invalid-level-missing-field.cmake | 18 +++ .../invalid-level-without-tu.cmake | 22 +++ .../invalid-missing-extents.cmake | 17 ++ .../dispatch-surface/invalid-no-extents.cmake | 17 ++ .../dispatch-surface/invalid-no-levels.cmake | 17 ++ .../invalid-non-numeric-extent.cmake | 17 ++ .../invalid-zero-extent.cmake | 18 +++ .../dispatch-surface/valid-minimal.cmake | 19 +++ .../dispatch-surface/valid-reduced.cmake | 25 +++ 19 files changed, 657 insertions(+), 100 deletions(-) create mode 100755 .github/scripts/check_dispatch_surface.sh create mode 100644 .github/workflows/dispatch-surface.yml create mode 100644 cmake/validate-dispatch-surface.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-duplicate-extent.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-duplicate-infix.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-duplicate-level.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-dynamic-listed.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-level-empty-field.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-level-missing-field.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-level-without-tu.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-missing-extents.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-no-extents.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-no-levels.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-non-numeric-extent.cmake create mode 100644 tests/cmake/dispatch-surface/invalid-zero-extent.cmake create mode 100644 tests/cmake/dispatch-surface/valid-minimal.cmake create mode 100644 tests/cmake/dispatch-surface/valid-reduced.cmake diff --git a/.github/scripts/check_dispatch_surface.sh b/.github/scripts/check_dispatch_surface.sh new file mode 100755 index 000000000..f9d19a15f --- /dev/null +++ b/.github/scripts/check_dispatch_surface.sh @@ -0,0 +1,111 @@ +#!/bin/bash +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Runs cmake/validate-dispatch-surface.cmake over the fixtures in +# tests/cmake/dispatch-surface and checks each verdict. +# +# valid-*.cmake must be accepted. +# invalid-*.cmake must be rejected, with a message containing the substring +# given by that fixture's `# EXPECT-ERROR:` line. +# +# Needs nothing but cmake -- no compiler, no dependencies, no build directory. + +set -uo pipefail + +root=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +fixtures="${root}/tests/cmake/dispatch-surface" +validator="${root}/cmake/validate-dispatch-surface.cmake" +x86_src_dir="${root}/include/svs/multi-arch/x86" + +if [[ ! -d ${fixtures} ]]; then + echo "no fixture directory: ${fixtures}" >&2 + exit 1 +fi + +# The default declaration must itself be valid -- checked as its own case so that +# a broken default is reported here rather than only at configure time. +run_validator() { + cmake "-DSVS_DISPATCH_SURFACE_FILE=$1" "-DSVS_X86_SRC_DIR=${x86_src_dir}" \ + -P "${validator}" 2>&1 +} + +# CMake indents and line-wraps error text, so compare against a whitespace- +# collapsed copy of the output. +flatten() { tr '\n' ' ' | tr -s '[:space:]' ' '; } + +failures=0 +checked=0 + +check_accepted() { + local fixture=$1 name=$2 output status + output=$(run_validator "${fixture}") + status=$? + if ((status != 0)); then + echo "FAIL ${name}: expected to be accepted, but validation failed:" >&2 + echo "${output}" | sed 's/^/ /' >&2 + ((failures++)) + else + echo "ok ${name}: accepted" + fi + ((checked++)) +} + +check_rejected() { + local fixture=$1 name=$2 expected output status + expected=$(sed -n 's/^# EXPECT-ERROR: *//p' "${fixture}") + if [[ -z ${expected} ]]; then + echo "FAIL ${name}: fixture has no '# EXPECT-ERROR:' line" >&2 + ((failures++)) + ((checked++)) + return + fi + + output=$(run_validator "${fixture}") + status=$? + if ((status == 0)); then + echo "FAIL ${name}: expected rejection, but validation succeeded" >&2 + ((failures++)) + elif [[ $(printf '%s' "${output}" | flatten) != *"${expected}"* ]]; then + echo "FAIL ${name}: rejected, but not for the stated reason." >&2 + echo " expected: ${expected}" >&2 + echo "${output}" | sed 's/^/ actual: /' >&2 + ((failures++)) + else + echo "ok ${name}: rejected (${expected})" + fi + ((checked++)) +} + +check_accepted "${root}/cmake/dispatch-surface.cmake" "dispatch-surface.cmake (default)" + +for fixture in "${fixtures}"/*.cmake; do + name=$(basename "${fixture}") + case ${name} in + valid-*) check_accepted "${fixture}" "${name}" ;; + invalid-*) check_rejected "${fixture}" "${name}" ;; + *) + echo "FAIL ${name}: fixture name must start with valid- or invalid-" >&2 + ((failures++)) + ((checked++)) + ;; + esac +done + +echo +if ((failures != 0)); then + echo "${failures} of ${checked} dispatch-surface checks failed" >&2 + exit 1 +fi +echo "all ${checked} dispatch-surface checks passed" diff --git a/.github/workflows/dispatch-surface.yml b/.github/workflows/dispatch-surface.yml new file mode 100644 index 000000000..dbe878a36 --- /dev/null +++ b/.github/workflows/dispatch-surface.yml @@ -0,0 +1,106 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# The set of distance kernels compiled ahead of time is declared once, in +# cmake/dispatch-surface.cmake, and generated from there. Two things have to stay +# true for that to be worth anything: the declaration must be checked rather than +# trusted, and it must be genuinely configurable -- a knob nobody turns is a knob +# that quietly stops working. + +name: Dispatch Surface + +on: + push: + branches: + - main + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }} + cancel-in-progress: true + +jobs: + declaration: + name: declaration is checked, committed header is current + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v6 + + # Every fixture in tests/cmake/dispatch-surface, plus the default + # declaration. Needs nothing but cmake. + - name: Accept and reject declarations + run: .github/scripts/check_dispatch_surface.sh + + # include/svs/core/distance/dispatch_surface.h is generated but committed, so + # that a bare `-I include` compile works without cmake. A configure refreshes + # it; if that produces a diff, either the declaration changed without a + # reconfigure or the header was edited by hand. + - name: Configure with the default surface + run: | + cmake -B "${{ runner.temp }}/build" -S "${GITHUB_WORKSPACE}" \ + -DCMAKE_BUILD_TYPE=Release \ + -DSVS_BUILD_TESTS=NO \ + -DSVS_BUILD_BINARIES=NO + + - name: Committed header matches the declaration + run: | + if ! git diff --exit-code -- include/svs/core/distance/dispatch_surface.h; then + echo "::error::include/svs/core/distance/dispatch_surface.h is stale." \ + "It is generated from cmake/dispatch-surface.cmake -- re-run cmake" \ + "and commit the result. Do not edit it by hand." + exit 1 + fi + + non-default-surface: + name: builds and tests with a non-default surface + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v6 + + # valid-reduced.cmake shares no extent with the default declaration, so a + # build that silently fell back to the committed header would fail to + # compile rather than pass by accident. + - name: Configure + run: | + cmake -B "${{ runner.temp }}/build" -S "${GITHUB_WORKSPACE}" \ + -DCMAKE_BUILD_TYPE=Release \ + -DSVS_BUILD_TESTS=YES \ + -DSVS_BUILD_BINARIES=NO \ + -DSVS_DISPATCH_SURFACE_FILE="${GITHUB_WORKSPACE}/tests/cmake/dispatch-surface/valid-reduced.cmake" + + - name: Build + working-directory: ${{ runner.temp }}/build + run: make -j$(nproc) + + # Correctness must not depend on which extents have a fixed-extent kernel: + # an extent without one is served by the svs::Dynamic kernel instead. The + # long-running tests are covered by the default-surface build. + - name: Run tests + env: + CTEST_OUTPUT_ON_FAILURE: 1 + working-directory: ${{ runner.temp }}/build/tests + run: ctest -C Release -LE long + + # Overriding the surface for one build must not rewrite the committed header. + - name: Committed header was left alone + run: | + if ! git diff --exit-code -- include/svs/core/distance/dispatch_surface.h; then + echo "::error::A build with an overridden dispatch surface rewrote the" \ + "committed header. Only the default declaration may refresh it." + exit 1 + fi diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index baf697990..f2772dc6f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,3 +34,14 @@ repos: args: [--markdown-linebreak-ext=md] exclude: .*\.svg$ - id: mixed-line-ending + + - repo: local + hooks: + # Cheap enough to run on every touch of the declaration, and it needs + # nothing but cmake -- no compiler, no build directory. + - id: dispatch-surface + name: dispatch surface declaration + entry: .github/scripts/check_dispatch_surface.sh + language: script + pass_filenames: false + files: ^(cmake/(dispatch-surface|validate-dispatch-surface)\.cmake|tests/cmake/dispatch-surface/.*\.cmake|\.github/scripts/check_dispatch_surface\.sh)$ diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake index 4aed2467e..9186639ef 100644 --- a/cmake/generate-dispatch-surface.cmake +++ b/cmake/generate-dispatch-surface.cmake @@ -27,12 +27,11 @@ set(SVS_DISPATCH_SURFACE_FILE "${SVS_DEFAULT_DISPATCH_SURFACE_FILE}" CACHE FILEPATH "Declaration of the ahead-of-time distance-kernel dispatch surface" ) -if(NOT EXISTS "${SVS_DISPATCH_SURFACE_FILE}") - message(FATAL_ERROR - "SVS_DISPATCH_SURFACE_FILE does not exist: ${SVS_DISPATCH_SURFACE_FILE}" - ) -endif() -include("${SVS_DISPATCH_SURFACE_FILE}") + +# Reads the declaration and rejects it if it is malformed. Also runnable on its +# own -- see .github/scripts/check_dispatch_surface.sh. +set(SVS_X86_SRC_DIR "${PROJECT_SOURCE_DIR}/include/svs/multi-arch/x86") +include("${CMAKE_CURRENT_LIST_DIR}/validate-dispatch-surface.cmake") file(REAL_PATH "${SVS_DISPATCH_SURFACE_FILE}" svs_surface_real) file(REAL_PATH "${SVS_DEFAULT_DISPATCH_SURFACE_FILE}" svs_default_surface_real) @@ -54,121 +53,38 @@ set_property( ) ##### -##### Validate the extent list -##### - -if(NOT SVS_SUPPORTED_DIMS) - message(FATAL_ERROR - "SVS_SUPPORTED_DIMS is empty in ${SVS_DISPATCH_SURFACE_FILE}. At least " - "one fixed extent is required." - ) -endif() - -foreach(dim IN LISTS SVS_SUPPORTED_DIMS) - if(NOT dim MATCHES "^[1-9][0-9]*$") - message(FATAL_ERROR - "SVS_SUPPORTED_DIMS contains '${dim}', which is not a positive " - "integer. svs::Dynamic is required and is appended automatically, " - "so it must not be listed." - ) - endif() -endforeach() - -set(svs_dims_sorted ${SVS_SUPPORTED_DIMS}) -list(REMOVE_DUPLICATES svs_dims_sorted) -list(LENGTH SVS_SUPPORTED_DIMS svs_dims_given) -list(LENGTH svs_dims_sorted svs_dims_unique) -if(NOT svs_dims_given EQUAL svs_dims_unique) - message(FATAL_ERROR - "SVS_SUPPORTED_DIMS contains duplicate extents. Every extent must " - "appear exactly once." - ) -endif() - -# svs::Dynamic is mandatory: it is what serves every dimensionality without a -# fixed-extent kernel, and the library is incorrect without it. -set(svs_dim_list ${SVS_SUPPORTED_DIMS} "svs::Dynamic") -list(LENGTH svs_dim_list SVS_GEN_DIM_COUNT) - -##### -##### Validate the ISA levels -##### - -if(NOT SVS_ISA_LEVELS) - message(FATAL_ERROR "SVS_ISA_LEVELS is empty in ${SVS_DISPATCH_SURFACE_FILE}.") -endif() - -set(svs_seen_levels) -set(svs_seen_infixes) -foreach(level_spec IN LISTS SVS_ISA_LEVELS) - string(REPLACE "|" ";" level_fields "${level_spec}") - list(LENGTH level_fields nfields) - if(NOT nfields EQUAL 3) - message(FATAL_ERROR - "Malformed SVS_ISA_LEVELS entry '${level_spec}': expected exactly " - "three '|'-separated fields ||." - ) - endif() - list(GET level_fields 0 level) - list(GET level_fields 1 arch) - list(GET level_fields 2 infix) - foreach(field level arch infix) - if(NOT ${field}) - message(FATAL_ERROR - "Malformed SVS_ISA_LEVELS entry '${level_spec}': ${field} is empty." - ) - endif() - endforeach() - if(level IN_LIST svs_seen_levels) - message(FATAL_ERROR "Duplicate ISA level '${level}' in SVS_ISA_LEVELS.") - endif() - if(infix IN_LIST svs_seen_infixes) - message(FATAL_ERROR - "Duplicate TU infix '${infix}' in SVS_ISA_LEVELS; infixes name " - "generated files and must be unique." - ) - endif() - list(APPEND svs_seen_levels ${level}) - list(APPEND svs_seen_infixes ${infix}) -endforeach() - -##### -##### Generate the header +##### Build the macro bodies ##### # Line continuations are emitted with a trailing backslash; the generated macros # are one logical line each. +set(SVS_GEN_DIM_COUNT ${SVS_DIM_COUNT}) + set(SVS_GEN_DIM_LOOP "\\\n") -foreach(dim IN LISTS svs_dim_list) +foreach(dim IN LISTS SVS_DIM_LIST) string(APPEND SVS_GEN_DIM_LOOP " M(${dim}) \\\n") endforeach() string(APPEND SVS_GEN_DIM_LOOP " /* end */") set(SVS_GEN_TARGET_LOOP "\\\n") set(SVS_DISPATCH_TU_SPECS) -set(svs_x86_src_dir "${PROJECT_SOURCE_DIR}/include/svs/multi-arch/x86") foreach(level_spec IN LISTS SVS_ISA_LEVELS) string(REPLACE "|" ";" level_fields "${level_spec}") list(GET level_fields 0 level) list(GET level_fields 1 arch) list(GET level_fields 2 infix) - foreach(dim IN LISTS svs_dim_list) + foreach(dim IN LISTS SVS_DIM_LIST) string(APPEND SVS_GEN_TARGET_LOOP " M(${dim}, ${level}) \\\n") endforeach() # One translation unit per level, named after the level's infix. The file # itself is short -- it loops over the generated extent list -- but it is - # committed rather than generated, because the private repository compiles - # these sources by path. - set(tu_src "${svs_x86_src_dir}/${infix}.cpp") - if(NOT EXISTS "${tu_src}") - message(FATAL_ERROR - "ISA level '${level}' has no translation unit: expected ${tu_src}. " - "Adding a level to SVS_ISA_LEVELS requires creating that file." - ) - endif() - list(APPEND SVS_DISPATCH_TU_SPECS "${tu_src}|${level}|${arch}|${infix}") + # committed rather than generated, because the downstream repository compiles + # these sources by path. Its existence was checked during validation. + list(APPEND SVS_DISPATCH_TU_SPECS + "${SVS_X86_SRC_DIR}/${infix}.cpp|${level}|${arch}|${infix}" + ) endforeach() string(APPEND SVS_GEN_TARGET_LOOP " /* end */") @@ -206,6 +122,6 @@ endif() list(LENGTH SVS_ISA_LEVELS svs_level_count) message(STATUS - "Dispatch surface: ${SVS_GEN_DIM_COUNT} extents (${svs_dims_unique} fixed + " + "Dispatch surface: ${SVS_DIM_COUNT} extents (${SVS_FIXED_DIM_COUNT} fixed + " "svs::Dynamic) x ${svs_level_count} ISA levels" ) diff --git a/cmake/validate-dispatch-surface.cmake b/cmake/validate-dispatch-surface.cmake new file mode 100644 index 000000000..fe9a06af5 --- /dev/null +++ b/cmake/validate-dispatch-surface.cmake @@ -0,0 +1,148 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +##### +##### Reads and checks a dispatch-surface declaration. +##### +##### This is deliberately free of build-system state so that it runs in script +##### mode as well as during configure: +##### +##### cmake -DSVS_DISPATCH_SURFACE_FILE= \ +##### -DSVS_X86_SRC_DIR= \ +##### -P cmake/validate-dispatch-surface.cmake +##### +##### Inputs: +##### SVS_DISPATCH_SURFACE_FILE -- the declaration to read +##### SVS_X86_SRC_DIR -- where per-level translation units live +##### +##### Outputs: +##### SVS_SUPPORTED_DIMS, SVS_ISA_LEVELS -- verbatim from the declaration +##### SVS_DIM_LIST -- extents, with svs::Dynamic appended +##### SVS_DIM_COUNT -- length of SVS_DIM_LIST +##### SVS_FIXED_DIM_COUNT -- length of SVS_SUPPORTED_DIMS +##### + +## In script mode there is no cmake_minimum_required, so policies default to OLD. +## Both of these are load-bearing here: CMP0007 keeps an empty `|`-field from +## vanishing when the entry is split, and CMP0057 enables `IN_LIST`. +cmake_policy(PUSH) +cmake_policy(SET CMP0007 NEW) +cmake_policy(SET CMP0057 NEW) + +if(NOT SVS_DISPATCH_SURFACE_FILE) + message(FATAL_ERROR "SVS_DISPATCH_SURFACE_FILE is not set.") +endif() +if(NOT EXISTS "${SVS_DISPATCH_SURFACE_FILE}") + message(FATAL_ERROR + "SVS_DISPATCH_SURFACE_FILE does not exist: ${SVS_DISPATCH_SURFACE_FILE}" + ) +endif() +if(NOT SVS_X86_SRC_DIR) + message(FATAL_ERROR "SVS_X86_SRC_DIR is not set.") +endif() + +# The declaration is plain CMake: it sets SVS_SUPPORTED_DIMS and SVS_ISA_LEVELS +# and does nothing else. Clear them first so that a declaration which forgets one +# is reported as empty rather than inheriting a value from the caller. +set(SVS_SUPPORTED_DIMS) +set(SVS_ISA_LEVELS) +include("${SVS_DISPATCH_SURFACE_FILE}") + +##### +##### The extent list +##### + +if(NOT SVS_SUPPORTED_DIMS) + message(FATAL_ERROR + "SVS_SUPPORTED_DIMS is empty in ${SVS_DISPATCH_SURFACE_FILE}. At least " + "one fixed extent is required." + ) +endif() + +foreach(dim IN LISTS SVS_SUPPORTED_DIMS) + if(NOT dim MATCHES "^[1-9][0-9]*$") + message(FATAL_ERROR + "SVS_SUPPORTED_DIMS contains '${dim}', which is not a positive " + "integer. svs::Dynamic is required and is appended automatically, " + "so it must not be listed." + ) + endif() +endforeach() + +set(svs_dims_deduped ${SVS_SUPPORTED_DIMS}) +list(REMOVE_DUPLICATES svs_dims_deduped) +list(LENGTH SVS_SUPPORTED_DIMS SVS_FIXED_DIM_COUNT) +list(LENGTH svs_dims_deduped svs_dims_unique) +if(NOT SVS_FIXED_DIM_COUNT EQUAL svs_dims_unique) + message(FATAL_ERROR + "SVS_SUPPORTED_DIMS contains duplicate extents. Every extent must " + "appear exactly once." + ) +endif() + +# svs::Dynamic is mandatory: it is what serves every dimensionality without a +# fixed-extent kernel, and the library is incorrect without it. +set(SVS_DIM_LIST ${SVS_SUPPORTED_DIMS} "svs::Dynamic") +list(LENGTH SVS_DIM_LIST SVS_DIM_COUNT) + +##### +##### The ISA levels +##### + +if(NOT SVS_ISA_LEVELS) + message(FATAL_ERROR "SVS_ISA_LEVELS is empty in ${SVS_DISPATCH_SURFACE_FILE}.") +endif() + +set(svs_seen_levels) +set(svs_seen_infixes) +foreach(level_spec IN LISTS SVS_ISA_LEVELS) + string(REPLACE "|" ";" level_fields "${level_spec}") + list(LENGTH level_fields nfields) + if(NOT nfields EQUAL 3) + message(FATAL_ERROR + "Malformed SVS_ISA_LEVELS entry '${level_spec}': expected exactly " + "three '|'-separated fields ||." + ) + endif() + list(GET level_fields 0 level) + list(GET level_fields 1 arch) + list(GET level_fields 2 infix) + foreach(field level arch infix) + if(NOT ${field}) + message(FATAL_ERROR + "Malformed SVS_ISA_LEVELS entry '${level_spec}': ${field} is empty." + ) + endif() + endforeach() + if(level IN_LIST svs_seen_levels) + message(FATAL_ERROR "Duplicate ISA level '${level}' in SVS_ISA_LEVELS.") + endif() + if(infix IN_LIST svs_seen_infixes) + message(FATAL_ERROR + "Duplicate TU infix '${infix}' in SVS_ISA_LEVELS; infixes name " + "generated files and must be unique." + ) + endif() + if(NOT EXISTS "${SVS_X86_SRC_DIR}/${infix}.cpp") + message(FATAL_ERROR + "ISA level '${level}' has no translation unit: expected " + "${SVS_X86_SRC_DIR}/${infix}.cpp. Adding a level to SVS_ISA_LEVELS " + "requires creating that file." + ) + endif() + list(APPEND svs_seen_levels ${level}) + list(APPEND svs_seen_infixes ${infix}) +endforeach() + +cmake_policy(POP) diff --git a/tests/cmake/dispatch-surface/invalid-duplicate-extent.cmake b/tests/cmake/dispatch-surface/invalid-duplicate-extent.cmake new file mode 100644 index 000000000..0d2efd2db --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-extent.cmake @@ -0,0 +1,17 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# EXPECT-ERROR: SVS_SUPPORTED_DIMS contains duplicate extents +set(SVS_SUPPORTED_DIMS 128 256 128) +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/invalid-duplicate-infix.cmake b/tests/cmake/dispatch-surface/invalid-duplicate-infix.cmake new file mode 100644 index 000000000..e7b76fb3e --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-infix.cmake @@ -0,0 +1,22 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Two levels cannot share one translation unit: it is compiled once, at one +# instruction budget. +# EXPECT-ERROR: Duplicate TU infix 'avx2' +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS + "AVX2|haswell|avx2" + "AVX512|cascadelake|avx2" +) diff --git a/tests/cmake/dispatch-surface/invalid-duplicate-level.cmake b/tests/cmake/dispatch-surface/invalid-duplicate-level.cmake new file mode 100644 index 000000000..0cd32fc8f --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-level.cmake @@ -0,0 +1,21 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# One enumerator cannot have two instruction budgets. +# EXPECT-ERROR: Duplicate ISA level 'AVX2' +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS + "AVX2|haswell|avx2" + "AVX2|cascadelake|avx512" +) diff --git a/tests/cmake/dispatch-surface/invalid-dynamic-listed.cmake b/tests/cmake/dispatch-surface/invalid-dynamic-listed.cmake new file mode 100644 index 000000000..ffe7d6d6d --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-dynamic-listed.cmake @@ -0,0 +1,18 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# svs::Dynamic is appended automatically and must not be spelled out. +# EXPECT-ERROR: svs::Dynamic is required and is appended automatically +set(SVS_SUPPORTED_DIMS 128 svs::Dynamic) +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/invalid-level-empty-field.cmake b/tests/cmake/dispatch-surface/invalid-level-empty-field.cmake new file mode 100644 index 000000000..2e6b83972 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-level-empty-field.cmake @@ -0,0 +1,17 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# EXPECT-ERROR: arch is empty +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS "AVX2||avx2") diff --git a/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake b/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake new file mode 100644 index 000000000..899632d9f --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake @@ -0,0 +1,18 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# The TU infix is absent. +# EXPECT-ERROR: expected exactly three '|'-separated fields +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS "AVX2|haswell") diff --git a/tests/cmake/dispatch-surface/invalid-level-without-tu.cmake b/tests/cmake/dispatch-surface/invalid-level-without-tu.cmake new file mode 100644 index 000000000..341258c99 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-level-without-tu.cmake @@ -0,0 +1,22 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Adding a level means adding the translation unit that defines its kernels. +# Without this check the level would silently contribute nothing. +# EXPECT-ERROR: has no translation unit +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS + "AVX2|haswell|avx2" + "AVX512VNNI|sapphirerapids|avx512vnni" +) diff --git a/tests/cmake/dispatch-surface/invalid-missing-extents.cmake b/tests/cmake/dispatch-surface/invalid-missing-extents.cmake new file mode 100644 index 000000000..eb35b6d3f --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-missing-extents.cmake @@ -0,0 +1,17 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# The variable is never set at all. +# EXPECT-ERROR: SVS_SUPPORTED_DIMS is empty +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/invalid-no-extents.cmake b/tests/cmake/dispatch-surface/invalid-no-extents.cmake new file mode 100644 index 000000000..df6b538ba --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-no-extents.cmake @@ -0,0 +1,17 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# EXPECT-ERROR: SVS_SUPPORTED_DIMS is empty +set(SVS_SUPPORTED_DIMS) +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/invalid-no-levels.cmake b/tests/cmake/dispatch-surface/invalid-no-levels.cmake new file mode 100644 index 000000000..f3297e809 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-no-levels.cmake @@ -0,0 +1,17 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# EXPECT-ERROR: SVS_ISA_LEVELS is empty +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS) diff --git a/tests/cmake/dispatch-surface/invalid-non-numeric-extent.cmake b/tests/cmake/dispatch-surface/invalid-non-numeric-extent.cmake new file mode 100644 index 000000000..21e2f98f4 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-non-numeric-extent.cmake @@ -0,0 +1,17 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# EXPECT-ERROR: SVS_SUPPORTED_DIMS contains '12a' +set(SVS_SUPPORTED_DIMS 128 12a) +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/invalid-zero-extent.cmake b/tests/cmake/dispatch-surface/invalid-zero-extent.cmake new file mode 100644 index 000000000..d68ae0b0b --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-zero-extent.cmake @@ -0,0 +1,18 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# A zero-length vector has no kernel to compile. +# EXPECT-ERROR: SVS_SUPPORTED_DIMS contains '0' +set(SVS_SUPPORTED_DIMS 0 128) +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/valid-minimal.cmake b/tests/cmake/dispatch-surface/valid-minimal.cmake new file mode 100644 index 000000000..f1ca55f8b --- /dev/null +++ b/tests/cmake/dispatch-surface/valid-minimal.cmake @@ -0,0 +1,19 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# The smallest declaration that is still a library: one fixed extent, one ISA +# level. Every other dimensionality is served by the svs::Dynamic kernel. + +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS "AVX2|haswell|avx2") diff --git a/tests/cmake/dispatch-surface/valid-reduced.cmake b/tests/cmake/dispatch-surface/valid-reduced.cmake new file mode 100644 index 000000000..e2410222f --- /dev/null +++ b/tests/cmake/dispatch-surface/valid-reduced.cmake @@ -0,0 +1,25 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# A surface that shares no extent with the default declaration, so a build using +# it cannot accidentally pass by reusing a committed header. Both ISA levels are +# kept so that runtime dispatch is still exercised on an AVX-512 host. +# +# Built and tested by the `non-default surface` CI job. + +set(SVS_SUPPORTED_DIMS 32 384) +set(SVS_ISA_LEVELS + "AVX2|haswell|avx2" + "AVX512|cascadelake|avx512" +) From bdd2303f7596023edf186796591b82ee9d8e3cb2 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Aug 2026 07:24:43 -0700 Subject: [PATCH 2/2] Add a link probe that checks the dispatch surface A kernel that is missing its `extern template` declaration does not produce an error. The consumer instantiates it locally instead, from the generic primary template -- and in a baseline consumer translation unit the vectorized partial specializations are not even visible, since they are guarded on SVS_AVX2 / SVS_AVX512_F. So the consumer silently gets a scalar loop where the library has a vectorized kernel, compiled at whatever -march the consumer happens to use. That is the bug that shipped for L2 at d=160 with AVX2. Nothing could catch it, because nothing referenced the whole surface at once. This adds a consumer that does: tests/multi-arch/x86/link_probe.cpp names every kernel the surface declares -- every (extent, ISA level) pair, every element-type pair, all three distances -- and nothing else. It is compiled at -march=x86-64, like an arbitrary consumer of the headers, and two tests are run against it: dispatch_surface_probe calls every kernel whose ISA level this host satisfies, so a kernel compiled beyond what its level guarantees faults here dispatch_surface_linkage reads the object's symbol table and requires the kernels it references to be exactly the kernels the library defines The linkage check is host-independent and covers the whole surface everywhere; the run covers only what the host can reach. On the default surface the two sets match exactly at 864 kernels, and on the reduced surface used by the non-default-surface CI job, at 288. All three failure modes were confirmed to fire: dropping the L2 extern block reports 288 kernels instantiated by the probe itself, and checking against an archive missing the AVX-512 translation unit reports its 432 kernels as declared but never instantiated. Co-Authored-By: Claude Opus 5 --- cmake/check-dispatch-linkage.cmake | 163 ++++++++++++++++++++++++++++ tests/CMakeLists.txt | 6 + tests/multi-arch/CMakeLists.txt | 69 ++++++++++++ tests/multi-arch/x86/link_probe.cpp | 120 ++++++++++++++++++++ 4 files changed, 358 insertions(+) create mode 100644 cmake/check-dispatch-linkage.cmake create mode 100644 tests/multi-arch/CMakeLists.txt create mode 100644 tests/multi-arch/x86/link_probe.cpp diff --git a/cmake/check-dispatch-linkage.cmake b/cmake/check-dispatch-linkage.cmake new file mode 100644 index 000000000..3eecaf7e8 --- /dev/null +++ b/cmake/check-dispatch-linkage.cmake @@ -0,0 +1,163 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +##### +##### Checks the dispatch surface against the symbols that were actually built. +##### +##### Run in script mode: +##### +##### cmake -DSVS_PROBE_OBJECT= \ +##### -DSVS_ARCHIVE= \ +##### -DSVS_NM= \ +##### -P cmake/check-dispatch-linkage.cmake +##### +##### The probe object names every kernel the surface declares and nothing else +##### (see tests/multi-arch/x86/link_probe.cpp), so the kernels it *references* +##### are exactly the kernels the archive must *define* -- and it must define no +##### others. Both directions are checked, plus that the probe defines none of +##### its own. +##### + +foreach(required SVS_PROBE_OBJECT SVS_ARCHIVE SVS_NM) + if(NOT ${required}) + message(FATAL_ERROR "${required} is not set.") + endif() +endforeach() +foreach(required SVS_PROBE_OBJECT SVS_ARCHIVE) + if(NOT EXISTS "${${required}}") + message(FATAL_ERROR "${required} does not exist: ${${required}}") + endif() +endforeach() + +# Distance kernels, and nothing else in the archive. Mangled names are used +# throughout: demangled ones carry `[clone .isra.0]` suffixes that differ between +# a local instantiation and an explicit one. +set(svs_kernel_regex "_ZN3svs8distance.*Impl") + +# Returns the mangled names of the matching symbols, one per list element. +function(svs_symbols out_var) + cmake_parse_arguments(arg "" "FILE" "NM_ARGS" ${ARGN}) + execute_process( + COMMAND "${SVS_NM}" ${arg_NM_ARGS} "${arg_FILE}" + OUTPUT_VARIABLE raw + ERROR_VARIABLE err + RESULT_VARIABLE status + ) + if(NOT status EQUAL 0) + message(FATAL_ERROR "${SVS_NM} failed on ${arg_FILE}: ${err}") + endif() + + set(symbols) + string(REPLACE "\n" ";" lines "${raw}") + foreach(line IN LISTS lines) + if(line MATCHES "${svs_kernel_regex}") + # The mangled name is the last whitespace-separated field. + string(REGEX MATCH "[^ \t]+$" symbol "${line}") + list(APPEND symbols "${symbol}") + endif() + endforeach() + list(REMOVE_DUPLICATES symbols) + list(SORT symbols) + set(${out_var} "${symbols}" PARENT_SCOPE) +endfunction() + +# Shows up to `limit` entries of a list. The names stay mangled -- pipe them +# through c++filt to read them. +function(svs_report_symbols symbols limit) + list(LENGTH symbols count) + set(shown ${symbols}) + if(count GREATER limit) + list(SUBLIST shown 0 ${limit} shown) + endif() + foreach(symbol IN LISTS shown) + message(" ${symbol}") + endforeach() + if(count GREATER limit) + math(EXPR rest "${count} - ${limit}") + message(" ... and ${rest} more") + endif() +endfunction() + +svs_symbols(probe_defines FILE "${SVS_PROBE_OBJECT}" NM_ARGS --defined-only) +svs_symbols(probe_references FILE "${SVS_PROBE_OBJECT}" NM_ARGS --undefined-only) +svs_symbols(archive_defines FILE "${SVS_ARCHIVE}" NM_ARGS --defined-only) + +list(LENGTH probe_defines n_probe_defines) +list(LENGTH probe_references n_probe_references) +list(LENGTH archive_defines n_archive_defines) + +set(errors 0) + +# A probe that names nothing is not a passing probe. This is what an LTO build +# looks like here, since the object holds IR rather than symbols. +if(n_probe_references EQUAL 0 AND n_probe_defines EQUAL 0) + message("${SVS_PROBE_OBJECT} names no distance kernels at all.") + message("Nothing can be concluded from it. If this is a link-time-optimized") + message("build, nm cannot see the symbols and this check does not apply.") + message(FATAL_ERROR "dispatch linkage check found no symbols to check") +endif() + +# The probe compiles at -march=x86-64 and guarantees nothing about the host, so a +# kernel it defines itself is a kernel some other consumer would also define +# itself -- at whatever -march that consumer happens to use. +if(NOT n_probe_defines EQUAL 0) + message("${n_probe_defines} kernels are instantiated by the probe itself.") + message("Each is missing its `extern template` declaration, so every consumer") + message("of the headers instantiates it locally, from the generic primary") + message("template, at the consumer's own -march. Declare them: the extern") + message("blocks in the distance headers must cover the whole surface.") + message(" Instantiated locally:") + svs_report_symbols("${probe_defines}" 10) + math(EXPR errors "${errors} + 1") +endif() + +# Declared but never instantiated. The link should already have failed, so this +# only fires when the object is inspected without being linked. +set(missing ${probe_references}) +if(archive_defines) + list(REMOVE_ITEM missing ${archive_defines}) +endif() +list(LENGTH missing n_missing) +if(NOT n_missing EQUAL 0) + message("${n_missing} kernels are declared but never instantiated.") + message("They are declared `extern template` in the distance headers but no") + message("translation unit defines them, so linking against the library fails.") + message(" Undefined:") + svs_report_symbols("${missing}" 10) + math(EXPR errors "${errors} + 1") +endif() + +# Instantiated but unreachable through the surface: dead weight in the archive. +set(unreachable ${archive_defines}) +if(probe_references) + list(REMOVE_ITEM unreachable ${probe_references}) +endif() +list(LENGTH unreachable n_unreachable) +if(NOT n_unreachable EQUAL 0) + message("${n_unreachable} kernels are instantiated but are not part of the") + message("dispatch surface. Nothing declares them, so no consumer reaches") + message("them; they only add to the size of the library.") + message(" Unreachable:") + svs_report_symbols("${unreachable}" 10) + math(EXPR errors "${errors} + 1") +endif() + +if(NOT errors EQUAL 0) + message(FATAL_ERROR "dispatch linkage check failed") +endif() + +message( + "dispatch linkage: ${n_archive_defines} kernels declared, instantiated and " + "reachable; none instantiated by the consumer" +) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8c812d35a..bb6d2a7d8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -237,3 +237,9 @@ list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras) include(CTest) include(Catch) catch_discover_tests(tests ADD_TAGS_AS_LABELS SKIP_IS_FAILURE) + +# Checks the x86 dispatch surface. The target only exists where the multi-arch +# build ran, which is the same condition that makes the surface meaningful. +if(TARGET svs_x86_objects) + add_subdirectory(multi-arch) +endif() diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt new file mode 100644 index 000000000..20380b7f7 --- /dev/null +++ b/tests/multi-arch/CMakeLists.txt @@ -0,0 +1,69 @@ +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +##### +##### The dispatch surface link probe. +##### +##### Not a Catch2 test: what is being checked is a property of the object file, +##### so the probe has to be its own translation unit, compiled at a known +##### instruction budget and inspected from outside. +##### + +# The object library exists so that the probe's single object file can be named +# with $, which is what the linkage check reads. +add_library(dispatch_surface_probe_objects OBJECT x86/link_probe.cpp) +target_link_libraries( + dispatch_surface_probe_objects + PRIVATE svs::svs svs::compile_options svs::x86_options_base +) + +# svs::x86_options_base is -march=x86-64 -mtune=generic: the probe must know no +# more about the host than an arbitrary consumer of the headers does. +add_executable(dispatch_surface_probe) +target_link_libraries( + dispatch_surface_probe + PRIVATE + dispatch_surface_probe_objects + svs::svs + svs::compile_options + svs::x86_options_base +) + +# Calls every kernel whose ISA level this host satisfies. Which kernels those are +# depends on the host, so this covers the whole surface only across the CI matrix. +add_test(NAME dispatch_surface_probe COMMAND dispatch_surface_probe) + +if(DEFINED CMAKE_NM AND CMAKE_NM) + set(svs_nm "${CMAKE_NM}") +else() + find_program(svs_nm NAMES nm llvm-nm) +endif() + +if(svs_nm) + # Host-independent, unlike the run above: it reads the symbol table rather + # than executing anything, so it sees every kernel in the surface everywhere. + add_test( + NAME dispatch_surface_linkage + COMMAND + "${CMAKE_COMMAND}" + "-DSVS_PROBE_OBJECT=$" + "-DSVS_ARCHIVE=$" + "-DSVS_NM=${svs_nm}" + # Not PROJECT_SOURCE_DIR: the downstream repository adds this directory + # to its own project, where that points somewhere else entirely. + -P "${CMAKE_CURRENT_LIST_DIR}/../../cmake/check-dispatch-linkage.cmake" + ) +else() + message(STATUS "nm not found; skipping the dispatch surface linkage test") +endif() diff --git a/tests/multi-arch/x86/link_probe.cpp b/tests/multi-arch/x86/link_probe.cpp new file mode 100644 index 000000000..f3c3d8a63 --- /dev/null +++ b/tests/multi-arch/x86/link_probe.cpp @@ -0,0 +1,120 @@ +/* + * Copyright 2025 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// A consumer that names every kernel the dispatch surface declares and nothing +// else, so a declared-but-uninstantiated kernel is an undefined symbol here. + +#include "svs/core/distance/cosine.h" +#include "svs/core/distance/euclidean.h" +#include "svs/core/distance/inner_product.h" +#include "svs/lib/avx_detection.h" +#include "svs/lib/static.h" +#include "svs/multi-arch/x86/preprocessor.h" + +#include +#include +#include +#include + +namespace { + +using svs::distance::AVX_AVAILABILITY; + +// A level added to the surface without a specialization here is an undefined +// symbol, deliberately: there is no way to guess the right predicate. +template bool host_satisfies(); + +template <> bool host_satisfies() { + return svs::detail::avx_runtime_flags.is_avx2_supported(); +} + +template <> bool host_satisfies() { + return svs::detail::avx_runtime_flags.is_avx512f_supported(); +} + +// The longest fixed extent in the surface. +constexpr size_t probe_max_dim = []() { + size_t longest = 1; + for (auto dim : svs::distance::supported_dim_list) { + if (dim != svs::Dynamic && dim > longest) { + longest = dim; + } + } + return longest; +}(); + +// A length for the svs::Dynamic kernels. Deliberately not a multiple of any +// vector width, so the epilogue is exercised too. +constexpr size_t probe_dynamic_dim = 97; + +// One buffer serves every call, whichever extents the surface happens to declare. +constexpr size_t probe_buffer_dim = std::max(probe_max_dim, probe_dynamic_dim); + +template const E* buffer() { + static const std::array values = []() { + std::array filled{}; + filled.fill(static_cast(1.0F)); + return filled; + }(); + return values.data(); +} + +// svs::lib::MaybeStatic has no default constructor: a dynamic +// extent must be told its length. +template svs::lib::MaybeStatic probe_length() { + if constexpr (N == svs::Dynamic) { + return svs::lib::MaybeStatic(probe_dynamic_dim); + } else { + return svs::lib::MaybeStatic(); + } +} + +// Named directly rather than through L2::compute, which also reaches +// AVX_AVAILABILITY::NONE -- not in the surface, so every consumer instantiates it. +#define SVS_PROBE_ONE(Ea, Eb, N, LEVEL) \ + total += svs::distance::L2Impl::compute( \ + buffer(), buffer(), probe_length() \ + ); \ + total += svs::distance::IPImpl::compute( \ + buffer(), buffer(), probe_length() \ + ); \ + total += \ + svs::distance::CosineSimilarityImpl::compute( \ + buffer(), buffer(), 1.0F, probe_length() \ + ); + +#define SVS_PROBE_TARGET(N, LEVEL) \ + if (host_satisfies()) { \ + SVS_FOR_EACH_TYPE_PAIR(SVS_PROBE_ONE, N, LEVEL) \ + } + +float probe_all() { + float total = 0; + SVS_FOR_EACH_DISPATCH_TARGET(SVS_PROBE_TARGET) + return total; +} + +#undef SVS_PROBE_TARGET +#undef SVS_PROBE_ONE + +} // namespace + +int main() { + // Printing keeps the calls above from being optimized away, and makes the run + // a smoke test of every kernel this host can reach. + std::printf("dispatch surface probe: %f\n", static_cast(probe_all())); + return 0; +}