From 3cf60410ee2ff8d1abc32b68ce9f0de05d534263 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Aug 2026 06:49:31 -0700 Subject: [PATCH 01/23] Generate the dispatch surface from a single declaration The set of distance kernels compiled ahead of time -- extents x ISA levels -- was written out by hand in every place that needed it: three extern template blocks, two per-arch translation units, the `supported_dim_list` array, and 48 near-identical `SPEC struct` lines in the instantiation macros. Adding an extent meant editing all of them and hoping none was missed. One had been: `euclidean.h` was missing d=160 for AVX2 (fixed in the preceding commit), which silently made consumers instantiate that kernel locally at their own -march. Declare the surface once, in `cmake/dispatch-surface.cmake`: set(SVS_SUPPORTED_DIMS 64 96 100 128 160 200 512 768) set(SVS_ISA_LEVELS "AVX2|haswell|avx2" "AVX512|cascadelake|avx512" ) `cmake/generate-dispatch-surface.cmake` validates it and writes `include/svs/core/distance/dispatch_surface.h`, which exports `SVS_FOR_EACH_SUPPORTED_DIM(M)`, `SVS_FOR_EACH_DISPATCH_TARGET(M)` and `SVS_SUPPORTED_DIM_COUNT`. Everything that used to spell the list out now loops over one of those. 108 hand-written instantiation lines become 0. Type pairs stay in C++, in `multi-arch/x86/preprocessor.h`. A pair exists because an implementation exists for it -- sometimes a hand-written one -- so the list belongs beside those implementations, not in the build system. `svs::Dynamic` is appended automatically and cannot be listed: it is what serves every dimensionality without a fixed-extent kernel, and the library is incorrect without it. The generated header is committed as well as generated. The build always compiles against the build-tree copy, placed ahead of the source include directory, and installs it over the committed one; the committed copy is refreshed only when the declaration is the default, so overriding the surface for a one-off build cannot rewrite the tree. Committing it keeps a bare `-I include` compile working without CMake -- which the downstream repository relies on, since it compiles `multi-arch/x86/{avx2,avx512}.cpp` by path with its own CMake. No behaviour change: the static library exports the same 864 symbols with the same sizes, and the two arch objects are symbol-identical before and after, both here and in the downstream build. `[distance]` passes (134402115 assertions, 13 test cases). Co-Authored-By: Claude Opus 5 --- CMakeLists.txt | 11 + cmake/dispatch-surface.cmake | 66 ++++++ cmake/generate-dispatch-surface.cmake | 211 +++++++++++++++++++ cmake/multi-arch.cmake | 30 +-- cmake/templates/dispatch_surface.h.in | 42 ++++ include/svs/core/distance/cosine.h | 25 +-- include/svs/core/distance/dispatch_surface.h | 71 +++++++ include/svs/core/distance/distance_core.h | 17 +- include/svs/core/distance/euclidean.h | 25 +-- include/svs/core/distance/inner_product.h | 26 +-- include/svs/multi-arch/x86/avx2.cpp | 36 +--- include/svs/multi-arch/x86/avx512.cpp | 36 +--- include/svs/multi-arch/x86/preprocessor.h | 127 +++++------ 13 files changed, 524 insertions(+), 199 deletions(-) create mode 100644 cmake/dispatch-surface.cmake create mode 100644 cmake/generate-dispatch-surface.cmake create mode 100644 cmake/templates/dispatch_surface.h.in create mode 100644 include/svs/core/distance/dispatch_surface.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 62377bef9..fcd429786 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -135,6 +135,17 @@ install( DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}" FILES_MATCHING PATTERN "*.h" ) + +# Install the generated dispatch-surface header over the committed one. They are +# identical for the default declaration; if the surface was overridden, the +# installed headers must describe what the library was actually built with. This +# must be declared after the directory install above so that it wins. +if(DEFINED SVS_GENERATED_DISPATCH_HEADER) + install( + FILES "${SVS_GENERATED_DISPATCH_HEADER}" + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/svs/core/distance" + ) +endif() install( EXPORT svs-targets NAMESPACE "svs::" diff --git a/cmake/dispatch-surface.cmake b/cmake/dispatch-surface.cmake new file mode 100644 index 000000000..341c89079 --- /dev/null +++ b/cmake/dispatch-surface.cmake @@ -0,0 +1,66 @@ +# Copyright 2026 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 for the x86 distance kernels. +##### +##### This file is the single place the extent list and the ISA levels are +##### written down. Everything derived from them is generated: +##### +##### - include/svs/core/distance/dispatch_surface.h, which drives every +##### `extern template` and explicit instantiation, and `supported_dim_list` +##### - the object library each ISA level's translation unit is compiled into, +##### and the instruction budget it is compiled at +##### +##### Edit this file. Do not edit the generated header; it is regenerated on +##### every configure and your changes there will be overwritten. +##### +##### A build may point somewhere else with -DSVS_DISPATCH_SURFACE_FILE=, +##### in which case the committed header is left alone and only the build tree +##### describes that surface. +##### + +# Extents that get their own fixed-extent kernel. +# +# This list is a *performance* choice, not a compatibility one. Any +# dimensionality not listed here still works and is fully supported: it +# dispatches to the `svs::Dynamic` kernel, which takes the length at run time. +# Listing an extent buys a fully unrolled kernel for it, at the cost of one more +# set of instantiations across every ISA level and type pair. +# +# `svs::Dynamic` is required and is appended automatically -- do not list it. +set(SVS_SUPPORTED_DIMS 64 96 100 128 160 200 512 768) + +# Runtime ISA levels. Each has one translation unit, which instantiates every +# extent above at that level. +# +# || +# +# enumerator a value of `svs::distance::AVX_AVAILABILITY` +# instruction budget the -march this level's kernels are compiled to; it must +# match what the level guarantees about the host, because +# anything the compiler is allowed to emit here will run on +# any host that satisfies the level +# TU infix names both the level's translation unit, +# include/svs/multi-arch/x86/.cpp, and the object +# library it is compiled into +# +# Adding a level here also requires a `SVS_TYPE_PAIRS_` list in +# include/svs/multi-arch/x86/preprocessor.h, saying which element-type pairs +# that level has kernels for. That is deliberately not configured here: a type +# pair exists because an implementation exists for it. +set(SVS_ISA_LEVELS + "AVX2|haswell|avx2" + "AVX512|cascadelake|avx512" +) diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake new file mode 100644 index 000000000..4aed2467e --- /dev/null +++ b/cmake/generate-dispatch-surface.cmake @@ -0,0 +1,211 @@ +# Copyright 2026 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. + +##### +##### Derives the dispatch surface declared in cmake/dispatch-surface.cmake. +##### +##### Produces: +##### include/svs/core/distance/dispatch_surface.h (source tree, committed) +##### SVS_DISPATCH_TU_SPECS -- "|||", one per ISA level +##### + +include_guard(GLOBAL) + +set(SVS_DEFAULT_DISPATCH_SURFACE_FILE "${CMAKE_CURRENT_LIST_DIR}/dispatch-surface.cmake") +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}") + +file(REAL_PATH "${SVS_DISPATCH_SURFACE_FILE}" svs_surface_real) +file(REAL_PATH "${SVS_DEFAULT_DISPATCH_SURFACE_FILE}" svs_default_surface_real) +if(svs_surface_real STREQUAL svs_default_surface_real) + set(svs_surface_is_default TRUE) +else() + set(svs_surface_is_default FALSE) + message(STATUS + "Dispatch surface overridden by ${SVS_DISPATCH_SURFACE_FILE}; the " + "committed header will not be refreshed" + ) +endif() + +# Re-run configure when the declaration changes, so the generated header and the +# translation units cannot go stale. +set_property( + DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS + "${SVS_DISPATCH_SURFACE_FILE}" +) + +##### +##### 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 +##### + +# Line continuations are emitted with a trailing backslash; the generated macros +# are one logical line each. +set(SVS_GEN_DIM_LOOP "\\\n") +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) + 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}") +endforeach() +string(APPEND SVS_GEN_TARGET_LOOP " /* end */") + +##### +##### Emit the header +##### + +# The build always compiles against the build-tree copy, and it is placed ahead +# of the source include directory so that it wins. +set(SVS_GENERATED_INCLUDE_DIR "${CMAKE_CURRENT_BINARY_DIR}/generated/include") +set(SVS_GENERATED_DISPATCH_HEADER + "${SVS_GENERATED_INCLUDE_DIR}/svs/core/distance/dispatch_surface.h" +) +configure_file( + "${CMAKE_CURRENT_LIST_DIR}/templates/dispatch_surface.h.in" + "${SVS_GENERATED_DISPATCH_HEADER}" + @ONLY +) +target_include_directories( + ${SVS_LIB} BEFORE INTERFACE $ +) + +# Refresh the committed copy too, but only for the default declaration: the +# committed header exists so that a bare `-I include` compile works without +# CMake, and a one-off build with an overridden surface must not rewrite it. +# configure_file only touches the file when the content changes, so this neither +# dirties the tree nor forces rebuilds. +if(svs_surface_is_default) + configure_file( + "${CMAKE_CURRENT_LIST_DIR}/templates/dispatch_surface.h.in" + "${PROJECT_SOURCE_DIR}/include/svs/core/distance/dispatch_surface.h" + @ONLY + ) +endif() + +list(LENGTH SVS_ISA_LEVELS svs_level_count) +message(STATUS + "Dispatch surface: ${SVS_GEN_DIM_COUNT} extents (${svs_dims_unique} fixed + " + "svs::Dynamic) x ${svs_level_count} ISA levels" +) diff --git a/cmake/multi-arch.cmake b/cmake/multi-arch.cmake index aeb81e693..74be6c57f 100644 --- a/cmake/multi-arch.cmake +++ b/cmake/multi-arch.cmake @@ -12,25 +12,29 @@ # See the License for the specific language governing permissions and # limitations under the License. -set(SVS_X86_SRC_DIR "${PROJECT_SOURCE_DIR}/include/svs/multi-arch/x86") -set(SVS_X86 - "${SVS_X86_SRC_DIR}/avx2.cpp,avx2,haswell" - "${SVS_X86_SRC_DIR}/avx512.cpp,avx512,cascadelake" -) +# Writes the generated dispatch-surface header and populates +# SVS_DISPATCH_TU_SPECS -- "|||", one entry per ISA +# level. The extent list and the levels themselves are declared in +# cmake/dispatch-surface.cmake. +include("${CMAKE_CURRENT_LIST_DIR}/generate-dispatch-surface.cmake") set(SVS_X86_OBJECT_FILES) -foreach(x86_info IN LISTS SVS_X86) - string(REPLACE "," ";" x86_info "${x86_info}") - list(GET x86_info 0 src) - list(GET x86_info 1 avx) - list(GET x86_info 2 arch) - set(lib_name "svs_x86_${avx}") +foreach(tu_spec IN LISTS SVS_DISPATCH_TU_SPECS) + string(REPLACE "|" ";" tu_fields "${tu_spec}") + list(GET tu_fields 0 src) + list(GET tu_fields 2 arch) + list(GET tu_fields 3 infix) + + # Carries the instruction budget for this level, and nothing else. + set(lib_name "svs_x86_${infix}") add_library(${lib_name} INTERFACE) target_compile_options(${lib_name} INTERFACE -march=${arch} -mtune=${arch}) - set(obj_name ${arch}_obj) + set(obj_name ${arch}_obj) add_library(${obj_name} OBJECT ${src}) - target_link_libraries(${obj_name} PRIVATE ${SVS_LIB} svs::compile_options fmt::fmt ${lib_name}) + target_link_libraries( + ${obj_name} PRIVATE ${SVS_LIB} svs::compile_options fmt::fmt ${lib_name} + ) list(APPEND SVS_X86_OBJECT_FILES $) endforeach() diff --git a/cmake/templates/dispatch_surface.h.in b/cmake/templates/dispatch_surface.h.in new file mode 100644 index 000000000..0047f3e68 --- /dev/null +++ b/cmake/templates/dispatch_surface.h.in @@ -0,0 +1,42 @@ +/* + * Copyright 2026 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. + */ + +// GENERATED FILE -- DO NOT EDIT. +// +// Regenerated on every CMake configure from cmake/dispatch-surface.cmake, which +// is where changes to the extent list or the ISA levels belong. This file is +// committed so that consuming the headers with a bare `-I include` -- no CMake +// -- keeps working. + +#pragma once + +// clang-format off +// +// The escaped-newline alignment column depends on the longest generated line, so +// letting clang-format reflow this file would make the committed copy disagree +// with the one CMake writes -- an endless format/regenerate loop. + +// Number of extents with a fixed-extent kernel, including svs::Dynamic. +#define SVS_SUPPORTED_DIM_COUNT @SVS_GEN_DIM_COUNT@ + +// Invokes M(extent) once per extent. +#define SVS_FOR_EACH_SUPPORTED_DIM(M) @SVS_GEN_DIM_LOOP@ + +// Invokes M(extent, isa_level) once per (extent, ISA level) pair -- that is, +// once per kernel the library compiles ahead of time, modulo type pairs. +#define SVS_FOR_EACH_DISPATCH_TARGET(M) @SVS_GEN_TARGET_LOOP@ + +// clang-format on diff --git a/include/svs/core/distance/cosine.h b/include/svs/core/distance/cosine.h index 9f4924997..4cd8e3780 100644 --- a/include/svs/core/distance/cosine.h +++ b/include/svs/core/distance/cosine.h @@ -500,26 +500,11 @@ struct CosineSimilarityImpl { #if defined(__x86_64__) #include "svs/multi-arch/x86/preprocessor.h" -// TODO: connect with dim_supported_list -DISTANCE_CS_EXTERN_TEMPLATE(64, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(96, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(100, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(128, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(160, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(200, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(512, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(768, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_EXTERN_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX512); - -DISTANCE_CS_EXTERN_TEMPLATE(64, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(96, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(100, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(128, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(160, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(200, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(512, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(768, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_EXTERN_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX2); +// Declare every kernel the library compiles ahead of time. Missing an entry here +// makes a consumer instantiate it locally, at the consumer's own -march. +#define SVS_CS_EXTERN(DIM, LEVEL) SVS_INSTANTIATE_CS(extern template, DIM, LEVEL) +SVS_FOR_EACH_DISPATCH_TARGET(SVS_CS_EXTERN) +#undef SVS_CS_EXTERN #endif } // namespace svs::distance diff --git a/include/svs/core/distance/dispatch_surface.h b/include/svs/core/distance/dispatch_surface.h new file mode 100644 index 000000000..2e10763e5 --- /dev/null +++ b/include/svs/core/distance/dispatch_surface.h @@ -0,0 +1,71 @@ +/* + * Copyright 2026 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. + */ + +// GENERATED FILE -- DO NOT EDIT. +// +// Regenerated on every CMake configure from cmake/dispatch-surface.cmake, which +// is where changes to the extent list or the ISA levels belong. This file is +// committed so that consuming the headers with a bare `-I include` -- no CMake +// -- keeps working. + +#pragma once + +// clang-format off +// +// The escaped-newline alignment column depends on the longest generated line, so +// letting clang-format reflow this file would make the committed copy disagree +// with the one CMake writes -- an endless format/regenerate loop. + +// Number of extents with a fixed-extent kernel, including svs::Dynamic. +#define SVS_SUPPORTED_DIM_COUNT 9 + +// Invokes M(extent) once per extent. +#define SVS_FOR_EACH_SUPPORTED_DIM(M) \ + M(64) \ + M(96) \ + M(100) \ + M(128) \ + M(160) \ + M(200) \ + M(512) \ + M(768) \ + M(svs::Dynamic) \ + /* end */ + +// Invokes M(extent, isa_level) once per (extent, ISA level) pair -- that is, +// once per kernel the library compiles ahead of time, modulo type pairs. +#define SVS_FOR_EACH_DISPATCH_TARGET(M) \ + M(64, AVX2) \ + M(96, AVX2) \ + M(100, AVX2) \ + M(128, AVX2) \ + M(160, AVX2) \ + M(200, AVX2) \ + M(512, AVX2) \ + M(768, AVX2) \ + M(svs::Dynamic, AVX2) \ + M(64, AVX512) \ + M(96, AVX512) \ + M(100, AVX512) \ + M(128, AVX512) \ + M(160, AVX512) \ + M(200, AVX512) \ + M(512, AVX512) \ + M(768, AVX512) \ + M(svs::Dynamic, AVX512) \ + /* end */ + +// clang-format on diff --git a/include/svs/core/distance/distance_core.h b/include/svs/core/distance/distance_core.h index 4f59f9de0..95ed06a74 100644 --- a/include/svs/core/distance/distance_core.h +++ b/include/svs/core/distance/distance_core.h @@ -21,6 +21,9 @@ #include "svs/lib/saveload.h" #include "svs/lib/type_traits.h" +// The extent list and the ISA levels, generated from cmake/dispatch-surface.cmake. +#include "svs/core/distance/dispatch_surface.h" + #include #include @@ -28,9 +31,17 @@ namespace svs::distance { enum class AVX_AVAILABILITY { NONE, AVX2, AVX512 }; -constexpr std::array supported_dim_list{ - 64, 96, 100, 128, 160, 200, 512, 768, svs::Dynamic}; - +/// The extents that have a fixed-extent kernel, including svs::Dynamic. +#define SVS_DIM_LIST_ENTRY(N) N, +constexpr std::array supported_dim_list{ + SVS_FOR_EACH_SUPPORTED_DIM(SVS_DIM_LIST_ENTRY)}; +#undef SVS_DIM_LIST_ENTRY + +/// Whether N has a fixed-extent kernel. +/// +/// This is not a capability test: every dimensionality is supported. An extent +/// that answers `false` here dispatches to the svs::Dynamic kernel instead of a +/// fully unrolled one. template constexpr bool is_dim_supported() { for (auto i : supported_dim_list) { if (i == N) { diff --git a/include/svs/core/distance/euclidean.h b/include/svs/core/distance/euclidean.h index 08aed5d25..a2fa68483 100644 --- a/include/svs/core/distance/euclidean.h +++ b/include/svs/core/distance/euclidean.h @@ -438,26 +438,11 @@ template struct L2Impl { #include "svs/multi-arch/x86/preprocessor.h" -// TODO: connect with dim_supported_list -DISTANCE_L2_EXTERN_TEMPLATE(64, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(96, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(100, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(128, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(160, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(200, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(512, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(768, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_EXTERN_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX512); - -DISTANCE_L2_EXTERN_TEMPLATE(64, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(96, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(100, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(128, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(160, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(200, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(512, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(768, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_EXTERN_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX2); +// Declare every kernel the library compiles ahead of time. Missing an entry here +// makes a consumer instantiate it locally, at the consumer's own -march. +#define SVS_L2_EXTERN(DIM, LEVEL) SVS_INSTANTIATE_L2(extern template, DIM, LEVEL) +SVS_FOR_EACH_DISPATCH_TARGET(SVS_L2_EXTERN) +#undef SVS_L2_EXTERN #endif } // namespace svs::distance diff --git a/include/svs/core/distance/inner_product.h b/include/svs/core/distance/inner_product.h index 0f7837a53..14293cb38 100644 --- a/include/svs/core/distance/inner_product.h +++ b/include/svs/core/distance/inner_product.h @@ -388,26 +388,12 @@ template struct IPImpl { #if defined(__x86_64__) #include "svs/multi-arch/x86/preprocessor.h" -// TODO: connect with dim_supported_list -DISTANCE_IP_EXTERN_TEMPLATE(64, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(96, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(100, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(128, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(160, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(200, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(512, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(768, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_EXTERN_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX512); - -DISTANCE_IP_EXTERN_TEMPLATE(64, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(96, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(100, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(128, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(160, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(200, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(512, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(768, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_EXTERN_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX2); + +// Declare every kernel the library compiles ahead of time. Missing an entry here +// makes a consumer instantiate it locally, at the consumer's own -march. +#define SVS_IP_EXTERN(DIM, LEVEL) SVS_INSTANTIATE_IP(extern template, DIM, LEVEL) +SVS_FOR_EACH_DISPATCH_TARGET(SVS_IP_EXTERN) +#undef SVS_IP_EXTERN #endif } // namespace svs::distance diff --git a/include/svs/multi-arch/x86/avx2.cpp b/include/svs/multi-arch/x86/avx2.cpp index bff53ae10..0142a2db9 100644 --- a/include/svs/multi-arch/x86/avx2.cpp +++ b/include/svs/multi-arch/x86/avx2.cpp @@ -21,36 +21,12 @@ namespace svs::distance { -// TODO: connect with dim_supported_list -DISTANCE_L2_INSTANTIATE_TEMPLATE(64, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(96, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(100, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(128, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(160, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(200, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(512, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(768, AVX_AVAILABILITY::AVX2); -DISTANCE_L2_INSTANTIATE_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX2); - -DISTANCE_IP_INSTANTIATE_TEMPLATE(64, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(96, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(100, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(128, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(160, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(200, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(512, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(768, AVX_AVAILABILITY::AVX2); -DISTANCE_IP_INSTANTIATE_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX2); - -DISTANCE_CS_INSTANTIATE_TEMPLATE(64, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(96, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(100, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(128, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(160, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(200, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(512, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(768, AVX_AVAILABILITY::AVX2); -DISTANCE_CS_INSTANTIATE_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX2); +// Define every kernel for this ISA level, at every extent in the generated +// list. The extents come from cmake/dispatch-surface.cmake; the type pairs come +// from svs/multi-arch/x86/preprocessor.h. +#define SVS_DEFINE_FOR_DIM(DIM) SVS_INSTANTIATE_DISTANCES(template, DIM, AVX2) +SVS_FOR_EACH_SUPPORTED_DIM(SVS_DEFINE_FOR_DIM) +#undef SVS_DEFINE_FOR_DIM } // namespace svs::distance diff --git a/include/svs/multi-arch/x86/avx512.cpp b/include/svs/multi-arch/x86/avx512.cpp index bee150d75..554e74561 100644 --- a/include/svs/multi-arch/x86/avx512.cpp +++ b/include/svs/multi-arch/x86/avx512.cpp @@ -21,36 +21,12 @@ namespace svs::distance { -// TODO: connect with dim_supported_list -DISTANCE_L2_INSTANTIATE_TEMPLATE(64, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(96, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(100, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(128, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(160, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(200, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(512, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(768, AVX_AVAILABILITY::AVX512); -DISTANCE_L2_INSTANTIATE_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX512); - -DISTANCE_IP_INSTANTIATE_TEMPLATE(64, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(96, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(100, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(128, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(160, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(200, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(512, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(768, AVX_AVAILABILITY::AVX512); -DISTANCE_IP_INSTANTIATE_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX512); - -DISTANCE_CS_INSTANTIATE_TEMPLATE(64, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(96, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(100, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(128, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(160, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(200, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(512, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(768, AVX_AVAILABILITY::AVX512); -DISTANCE_CS_INSTANTIATE_TEMPLATE(Dynamic, AVX_AVAILABILITY::AVX512); +// Define every kernel for this ISA level, at every extent in the generated +// list. The extents come from cmake/dispatch-surface.cmake; the type pairs come +// from svs/multi-arch/x86/preprocessor.h. +#define SVS_DEFINE_FOR_DIM(DIM) SVS_INSTANTIATE_DISTANCES(template, DIM, AVX512) +SVS_FOR_EACH_SUPPORTED_DIM(SVS_DEFINE_FOR_DIM) +#undef SVS_DEFINE_FOR_DIM } // namespace svs::distance diff --git a/include/svs/multi-arch/x86/preprocessor.h b/include/svs/multi-arch/x86/preprocessor.h index 4e0cb941d..9b8157f40 100644 --- a/include/svs/multi-arch/x86/preprocessor.h +++ b/include/svs/multi-arch/x86/preprocessor.h @@ -16,74 +16,75 @@ #pragma once -#define DISTANCE_L2_TEMPLATE_HELPER(SPEC, N, AVX) \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; \ - SPEC struct L2Impl; +#include "svs/core/distance/dispatch_surface.h" -#define DISTANCE_L2_INSTANTIATE_TEMPLATE(N, AVX) \ - DISTANCE_L2_TEMPLATE_HELPER(template, N, AVX); +///// +///// Element-type pairs. +///// +///// Hand-written, and deliberately so. Unlike the extent list -- which is +///// declared in cmake/dispatch-surface.cmake and generated -- a type pair is +///// not a free axis: it appears here because a kernel exists for it, in some +///// cases a hand-crafted specialization. Generating this list would invite +///// combinations with no implementation to reach. +///// -#define DISTANCE_L2_EXTERN_TEMPLATE(N, AVX) \ - DISTANCE_L2_TEMPLATE_HELPER(extern template, N, AVX); +// Invokes M(query_type, dataset_type, ...) once per type pair. +#define SVS_FOR_EACH_TYPE_PAIR(M, ...) \ + M(float, float, __VA_ARGS__) \ + M(float, int8_t, __VA_ARGS__) \ + M(float, uint8_t, __VA_ARGS__) \ + M(float, svs::float16::Float16, __VA_ARGS__) \ + M(int8_t, float, __VA_ARGS__) \ + M(int8_t, int8_t, __VA_ARGS__) \ + M(int8_t, uint8_t, __VA_ARGS__) \ + M(int8_t, svs::float16::Float16, __VA_ARGS__) \ + M(uint8_t, float, __VA_ARGS__) \ + M(uint8_t, int8_t, __VA_ARGS__) \ + M(uint8_t, uint8_t, __VA_ARGS__) \ + M(uint8_t, svs::float16::Float16, __VA_ARGS__) \ + M(svs::float16::Float16, float, __VA_ARGS__) \ + M(svs::float16::Float16, int8_t, __VA_ARGS__) \ + M(svs::float16::Float16, uint8_t, __VA_ARGS__) \ + M(svs::float16::Float16, svs::float16::Float16, __VA_ARGS__) -#define DISTANCE_IP_TEMPLATE_HELPER(SPEC, N, AVX) \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; \ - SPEC struct IPImpl; +///// +///// Which type pairs each ISA level has kernels for. +///// +///// One line per level in SVS_ISA_LEVELS. A level with no entry here is a +///// compile error rather than a silently empty instantiation list. +///// -#define DISTANCE_IP_INSTANTIATE_TEMPLATE(N, AVX) \ - DISTANCE_IP_TEMPLATE_HELPER(template, N, AVX); +#define SVS_TYPE_PAIRS_NONE SVS_FOR_EACH_TYPE_PAIR +#define SVS_TYPE_PAIRS_AVX2 SVS_FOR_EACH_TYPE_PAIR +#define SVS_TYPE_PAIRS_AVX512 SVS_FOR_EACH_TYPE_PAIR -#define DISTANCE_IP_EXTERN_TEMPLATE(N, AVX) \ - DISTANCE_IP_TEMPLATE_HELPER(extern template, N, AVX); +///// +///// Instantiation. +///// -#define DISTANCE_CS_TEMPLATE_HELPER(SPEC, N, AVX) \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; \ - SPEC struct CosineSimilarityImpl; +// Resolve LEVEL to its type-pair list. The indirection is required so that +// LEVEL is expanded before being pasted. +#define SVS_TYPE_PAIRS_FOR_(LEVEL) SVS_TYPE_PAIRS_##LEVEL +#define SVS_TYPE_PAIRS_FOR(LEVEL, M, ...) SVS_TYPE_PAIRS_FOR_(LEVEL)(M, __VA_ARGS__) -#define DISTANCE_CS_INSTANTIATE_TEMPLATE(N, AVX) \ - DISTANCE_CS_TEMPLATE_HELPER(template, N, AVX); +#define SVS_DECLARE_ONE_L2(Ea, Eb, SPEC, N, LEVEL) \ + SPEC struct L2Impl; +#define SVS_DECLARE_ONE_IP(Ea, Eb, SPEC, N, LEVEL) \ + SPEC struct IPImpl; +#define SVS_DECLARE_ONE_CS(Ea, Eb, SPEC, N, LEVEL) \ + SPEC struct CosineSimilarityImpl; -#define DISTANCE_CS_EXTERN_TEMPLATE(N, AVX) \ - DISTANCE_CS_TEMPLATE_HELPER(extern template, N, AVX); +// SPEC is `template` for a definition or `extern template` for a declaration. +#define SVS_INSTANTIATE_L2(SPEC, N, LEVEL) \ + SVS_TYPE_PAIRS_FOR(LEVEL, SVS_DECLARE_ONE_L2, SPEC, N, LEVEL) +#define SVS_INSTANTIATE_IP(SPEC, N, LEVEL) \ + SVS_TYPE_PAIRS_FOR(LEVEL, SVS_DECLARE_ONE_IP, SPEC, N, LEVEL) +#define SVS_INSTANTIATE_CS(SPEC, N, LEVEL) \ + SVS_TYPE_PAIRS_FOR(LEVEL, SVS_DECLARE_ONE_CS, SPEC, N, LEVEL) + +// All three distances at once. Only usable where all three are declared, i.e. +// in a generated translation unit. +#define SVS_INSTANTIATE_DISTANCES(SPEC, N, LEVEL) \ + SVS_INSTANTIATE_L2(SPEC, N, LEVEL) \ + SVS_INSTANTIATE_IP(SPEC, N, LEVEL) \ + SVS_INSTANTIATE_CS(SPEC, N, LEVEL) From e4cd87bed2d4f068c17afaa0ca189df9feafecc9 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Sun, 23 Aug 2026 23:54:47 -0700 Subject: [PATCH 02/23] docs(dispatch): tighten the new comments to two lines each Every comment this branch adds now says what the code cannot say for itself and stops there. The block comments that restated the surrounding code, or spent five lines on a hazard that takes two, are gone; the hazards themselves stay, each naming its failure mode. Comment-only. The non-comment diff against the previous tip is empty. --- cmake/templates/dispatch_surface.h.in | 14 ++++---------- include/svs/core/distance/dispatch_surface.h | 14 ++++---------- include/svs/core/distance/distance_core.h | 5 ++--- include/svs/multi-arch/x86/avx2.cpp | 5 ++--- include/svs/multi-arch/x86/avx512.cpp | 5 ++--- include/svs/multi-arch/x86/preprocessor.h | 7 ++----- 6 files changed, 16 insertions(+), 34 deletions(-) diff --git a/cmake/templates/dispatch_surface.h.in b/cmake/templates/dispatch_surface.h.in index 0047f3e68..01c1d5f17 100644 --- a/cmake/templates/dispatch_surface.h.in +++ b/cmake/templates/dispatch_surface.h.in @@ -14,20 +14,14 @@ * limitations under the License. */ -// GENERATED FILE -- DO NOT EDIT. -// -// Regenerated on every CMake configure from cmake/dispatch-surface.cmake, which -// is where changes to the extent list or the ISA levels belong. This file is -// committed so that consuming the headers with a bare `-I include` -- no CMake -// -- keeps working. +// GENERATED FILE -- DO NOT EDIT. Regenerated on every CMake configure from +// cmake/dispatch-surface.cmake; committed so a bare `-I include` compile works. #pragma once // clang-format off -// -// The escaped-newline alignment column depends on the longest generated line, so -// letting clang-format reflow this file would make the committed copy disagree -// with the one CMake writes -- an endless format/regenerate loop. +// Reflowing would realign the escaped newlines and make the committed copy +// disagree with the one CMake writes -- an endless format/regenerate loop. // Number of extents with a fixed-extent kernel, including svs::Dynamic. #define SVS_SUPPORTED_DIM_COUNT @SVS_GEN_DIM_COUNT@ diff --git a/include/svs/core/distance/dispatch_surface.h b/include/svs/core/distance/dispatch_surface.h index 2e10763e5..38bcc2853 100644 --- a/include/svs/core/distance/dispatch_surface.h +++ b/include/svs/core/distance/dispatch_surface.h @@ -14,20 +14,14 @@ * limitations under the License. */ -// GENERATED FILE -- DO NOT EDIT. -// -// Regenerated on every CMake configure from cmake/dispatch-surface.cmake, which -// is where changes to the extent list or the ISA levels belong. This file is -// committed so that consuming the headers with a bare `-I include` -- no CMake -// -- keeps working. +// GENERATED FILE -- DO NOT EDIT. Regenerated on every CMake configure from +// cmake/dispatch-surface.cmake; committed so a bare `-I include` compile works. #pragma once // clang-format off -// -// The escaped-newline alignment column depends on the longest generated line, so -// letting clang-format reflow this file would make the committed copy disagree -// with the one CMake writes -- an endless format/regenerate loop. +// Reflowing would realign the escaped newlines and make the committed copy +// disagree with the one CMake writes -- an endless format/regenerate loop. // Number of extents with a fixed-extent kernel, including svs::Dynamic. #define SVS_SUPPORTED_DIM_COUNT 9 diff --git a/include/svs/core/distance/distance_core.h b/include/svs/core/distance/distance_core.h index 95ed06a74..5d50dee1f 100644 --- a/include/svs/core/distance/distance_core.h +++ b/include/svs/core/distance/distance_core.h @@ -39,9 +39,8 @@ constexpr std::array supported_dim_list{ /// Whether N has a fixed-extent kernel. /// -/// This is not a capability test: every dimensionality is supported. An extent -/// that answers `false` here dispatches to the svs::Dynamic kernel instead of a -/// fully unrolled one. +/// Not a capability test: every dimensionality is supported. An extent answering +/// `false` dispatches to the svs::Dynamic kernel instead of a fully unrolled one. template constexpr bool is_dim_supported() { for (auto i : supported_dim_list) { if (i == N) { diff --git a/include/svs/multi-arch/x86/avx2.cpp b/include/svs/multi-arch/x86/avx2.cpp index 0142a2db9..7ee912684 100644 --- a/include/svs/multi-arch/x86/avx2.cpp +++ b/include/svs/multi-arch/x86/avx2.cpp @@ -21,9 +21,8 @@ namespace svs::distance { -// Define every kernel for this ISA level, at every extent in the generated -// list. The extents come from cmake/dispatch-surface.cmake; the type pairs come -// from svs/multi-arch/x86/preprocessor.h. +// Define every kernel for this ISA level at every generated extent. Extents come +// from cmake/dispatch-surface.cmake, type pairs from multi-arch/x86/preprocessor.h. #define SVS_DEFINE_FOR_DIM(DIM) SVS_INSTANTIATE_DISTANCES(template, DIM, AVX2) SVS_FOR_EACH_SUPPORTED_DIM(SVS_DEFINE_FOR_DIM) #undef SVS_DEFINE_FOR_DIM diff --git a/include/svs/multi-arch/x86/avx512.cpp b/include/svs/multi-arch/x86/avx512.cpp index 554e74561..c3cd44601 100644 --- a/include/svs/multi-arch/x86/avx512.cpp +++ b/include/svs/multi-arch/x86/avx512.cpp @@ -21,9 +21,8 @@ namespace svs::distance { -// Define every kernel for this ISA level, at every extent in the generated -// list. The extents come from cmake/dispatch-surface.cmake; the type pairs come -// from svs/multi-arch/x86/preprocessor.h. +// Define every kernel for this ISA level at every generated extent. Extents come +// from cmake/dispatch-surface.cmake, type pairs from multi-arch/x86/preprocessor.h. #define SVS_DEFINE_FOR_DIM(DIM) SVS_INSTANTIATE_DISTANCES(template, DIM, AVX512) SVS_FOR_EACH_SUPPORTED_DIM(SVS_DEFINE_FOR_DIM) #undef SVS_DEFINE_FOR_DIM diff --git a/include/svs/multi-arch/x86/preprocessor.h b/include/svs/multi-arch/x86/preprocessor.h index 9b8157f40..2f3f71af7 100644 --- a/include/svs/multi-arch/x86/preprocessor.h +++ b/include/svs/multi-arch/x86/preprocessor.h @@ -21,11 +21,8 @@ ///// ///// Element-type pairs. ///// -///// Hand-written, and deliberately so. Unlike the extent list -- which is -///// declared in cmake/dispatch-surface.cmake and generated -- a type pair is -///// not a free axis: it appears here because a kernel exists for it, in some -///// cases a hand-crafted specialization. Generating this list would invite -///// combinations with no implementation to reach. +///// Hand-written, unlike the generated extent list: a pair is here because a +///// kernel exists for it, and generating it would invite unimplemented pairs. ///// // Invokes M(query_type, dataset_type, ...) once per type pair. From ffa2a2108aa70baab832c436842b1d3ad3098996 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Aug 2026 07:24:43 -0700 Subject: [PATCH 03/23] 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..c8d542561 --- /dev/null +++ b/cmake/check-dispatch-linkage.cmake @@ -0,0 +1,163 @@ +# Copyright 2026 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..d7a82a288 --- /dev/null +++ b/tests/multi-arch/x86/link_probe.cpp @@ -0,0 +1,120 @@ +/* + * Copyright 2026 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; +} From e5447889540d5c27b4a3f218b203d0af2b544b63 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 24 Aug 2026 02:12:53 -0700 Subject: [PATCH 04/23] build(dispatch): spell out the extents and ISA levels at configure time "9 extents (8 fixed + svs::Dynamic) x 2 ISA levels" says nothing about which extents, which levels, or what instruction budget each level compiles at, so reading the log gave no way to tell a correct surface from a plausible one. Also name the AVX_AVAILABILITY enumerators that are not in the surface, since that is the question the old count invited and could not answer: NONE is dispatched to but has no translation unit, so every consumer instantiates its kernels itself, at the consumer's own -march. Co-Authored-By: Claude Opus 5 --- cmake/generate-dispatch-surface.cmake | 40 +++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake index 4aed2467e..a9bf2c385 100644 --- a/cmake/generate-dispatch-surface.cmake +++ b/cmake/generate-dispatch-surface.cmake @@ -169,6 +169,9 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) ) endif() list(APPEND SVS_DISPATCH_TU_SPECS "${tu_src}|${level}|${arch}|${infix}") + list(APPEND svs_level_report + "AVX_AVAILABILITY::${level} -march=${arch} ${infix}.cpp" + ) endforeach() string(APPEND SVS_GEN_TARGET_LOOP " /* end */") @@ -204,8 +207,41 @@ if(svs_surface_is_default) ) endif() +##### +##### Report the surface +##### + list(LENGTH SVS_ISA_LEVELS svs_level_count) +string(REPLACE ";" " " svs_dims_display "${SVS_SUPPORTED_DIMS}") message(STATUS - "Dispatch surface: ${SVS_GEN_DIM_COUNT} extents (${svs_dims_unique} fixed + " - "svs::Dynamic) x ${svs_level_count} ISA levels" + "Dispatch surface: ${SVS_GEN_DIM_COUNT} extents x ${svs_level_count} ISA levels" ) +message(STATUS " extents: ${svs_dims_display} svs::Dynamic") +foreach(entry IN LISTS svs_level_report) + message(STATUS " level: ${entry}") +endforeach() + +# Every enumerator without a translation unit is still reachable -- the entry +# points fall back to it -- so its kernels are built by each consumer instead. +set(svs_enum_header "${PROJECT_SOURCE_DIR}/include/svs/core/distance/distance_core.h") +if(EXISTS "${svs_enum_header}") + file(READ "${svs_enum_header}" svs_enum_text) + if(svs_enum_text MATCHES "enum class AVX_AVAILABILITY[ \t\r\n]*{([^}]*)}") + string(REPLACE "," ";" svs_enumerators "${CMAKE_MATCH_1}") + set(svs_undeclared) + foreach(enumerator IN LISTS svs_enumerators) + string(STRIP "${enumerator}" enumerator) + if(enumerator AND NOT enumerator IN_LIST svs_seen_levels) + list(APPEND svs_undeclared "${enumerator}") + endif() + endforeach() + if(svs_undeclared) + string(REPLACE ";" ", " svs_undeclared_display "${svs_undeclared}") + message(STATUS " not in the surface: ${svs_undeclared_display}") + message(STATUS + " dispatched to, but compiled by no translation unit, so " + "every consumer instantiates those kernels itself, at its own -march" + ) + endif() + endif() +endif() From 75cf382b956ced4e96bf37206881840126606b7f Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 24 Aug 2026 02:13:06 -0700 Subject: [PATCH 05/23] test(dispatch): check the built surface against its declaration Four checks, each closing a failure mode the link probe cannot see. dispatch_surface_declaration derives what the library must contain from the three hand-written sources -- the extent list and ISA levels, the type-pair lists, and the AVX_AVAILABILITY enumerator order -- and never reads the generated header. The linkage check compares the archive against a probe built from that header, so a generator that dropped an extent would drop it from both and still agree; this one has nowhere to hide. It also checks the entry-point consumer, whose kernels must all come from the archive: one it defines itself is an extern declaration that is missing. dispatch_instructions_, one test per ISA level, disassembles the level's object file and holds it to a budget table keyed by -march. A level guarantees only what its runtime predicate tests, so an instruction outside that budget faults on a host the dispatcher routes there -- and no symbol-table check can see it. dispatch_surface_execution is the only check that observes a kernel run rather than exist: a specialization lost behind an `#if` still links and still counts. It breaks on every level's kernel for one extent and confirms the run enters the level this host satisfies. Weaker levels are covered by hosts that satisfy only those. dispatch_entry_probe reaches the kernels through the entry points rather than by naming the Impl classes, which is what makes the consumer half of the declaration check meaningful. nm, objdump and gdb are each optional: a missing tool skips its tests rather than failing the build. Co-Authored-By: Claude Opus 5 --- cmake/check-dispatch-declaration.cmake | 345 +++++++++++++++++++ cmake/check-dispatch-execution.cmake | 155 +++++++++ cmake/check-dispatch-instructions.cmake | 148 ++++++++ cmake/generate-dispatch-surface.cmake | 3 + cmake/templates/dispatch_surface.h.in | 4 + include/svs/core/distance/dispatch_surface.h | 7 + tests/multi-arch/CMakeLists.txt | 127 +++++-- tests/multi-arch/x86/entry_probe.cpp | 112 ++++++ tests/multi-arch/x86/host_levels.h | 56 +++ tests/multi-arch/x86/link_probe.cpp | 16 +- 10 files changed, 932 insertions(+), 41 deletions(-) create mode 100644 cmake/check-dispatch-declaration.cmake create mode 100644 cmake/check-dispatch-execution.cmake create mode 100644 cmake/check-dispatch-instructions.cmake create mode 100644 tests/multi-arch/x86/entry_probe.cpp create mode 100644 tests/multi-arch/x86/host_levels.h diff --git a/cmake/check-dispatch-declaration.cmake b/cmake/check-dispatch-declaration.cmake new file mode 100644 index 000000000..20d5c4c1d --- /dev/null +++ b/cmake/check-dispatch-declaration.cmake @@ -0,0 +1,345 @@ +# Copyright 2026 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 built kernels against the declaration, not against the generated +##### header. +##### +##### Run in script mode: +##### +##### cmake -DSVS_SURFACE_FILE=cmake/dispatch-surface.cmake \ +##### -DSVS_TYPE_PAIR_HEADER=include/svs/multi-arch/x86/preprocessor.h \ +##### -DSVS_ENUM_HEADER=include/svs/core/distance/distance_core.h \ +##### -DSVS_ARCHIVE= \ +##### -DSVS_CONSUMER_OBJECT= \ +##### -DSVS_NM= \ +##### -P cmake/check-dispatch-declaration.cmake +##### +##### cmake/check-dispatch-linkage.cmake compares the archive against a probe, and +##### both are generated from the same header: a generator that dropped an extent +##### would drop it from both and still agree. The expectation here is derived from +##### the three hand-written sources instead -- the extent list and levels, the type +##### pairs, and the enumerator order -- so the generated header is not consulted at +##### all and a generator bug has nowhere to hide. +##### + +# Without it, CMP0057 is unset in script mode and the `IN_LIST` tests below are +# rejected as unknown arguments. +cmake_minimum_required(VERSION 3.21) + +foreach(required + SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER + SVS_ARCHIVE SVS_CONSUMER_OBJECT SVS_NM +) + if(NOT ${required}) + message(FATAL_ERROR "${required} is not set.") + endif() +endforeach() +foreach(required + SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER + SVS_ARCHIVE SVS_CONSUMER_OBJECT +) + if(NOT EXISTS "${${required}}") + message(FATAL_ERROR "${required} does not exist: ${${required}}") + endif() +endforeach() + +##### +##### Ground truth 1: the extents and the ISA levels +##### + +include("${SVS_SURFACE_FILE}") + +if(NOT SVS_SUPPORTED_DIMS OR NOT SVS_ISA_LEVELS) + message(FATAL_ERROR + "${SVS_SURFACE_FILE} declares no extents or no ISA levels; it is not a " + "dispatch-surface declaration." + ) +endif() + +# svs::Dynamic mangles as its numeric value: the extent is a size_t template +# argument, and Dynamic is SIZE_MAX. +set(svs_expected_extents ${SVS_SUPPORTED_DIMS} "18446744073709551615") + +set(svs_expected_levels) +foreach(level_spec IN LISTS SVS_ISA_LEVELS) + string(REPLACE "|" ";" fields "${level_spec}") + list(GET fields 0 level) + list(APPEND svs_expected_levels "${level}") +endforeach() + +##### +##### Ground truth 2: the enumerator order, which fixes the mangled digit +##### + +file(READ "${SVS_ENUM_HEADER}" svs_enum_text) +if(NOT svs_enum_text MATCHES "enum class AVX_AVAILABILITY[ \t\r\n]*{([^}]*)}") + message(FATAL_ERROR + "No `enum class AVX_AVAILABILITY` found in ${SVS_ENUM_HEADER}. Its " + "enumerator order is what maps an ISA level onto the digit in a mangled " + "name, so it cannot be inferred." + ) +endif() +string(REPLACE "," ";" svs_enumerators "${CMAKE_MATCH_1}") +set(svs_enum_index 0) +foreach(enumerator IN LISTS svs_enumerators) + string(STRIP "${enumerator}" enumerator) + if(enumerator) + set(svs_digit_of_${enumerator} ${svs_enum_index}) + math(EXPR svs_enum_index "${svs_enum_index} + 1") + endif() +endforeach() + +set(svs_expected_digits) +foreach(level IN LISTS svs_expected_levels) + if(NOT DEFINED svs_digit_of_${level}) + message(FATAL_ERROR + "ISA level '${level}' is declared in ${SVS_SURFACE_FILE} but is not an " + "AVX_AVAILABILITY enumerator." + ) + endif() + list(APPEND svs_expected_digits ${svs_digit_of_${level}}) +endforeach() + +##### +##### Ground truth 3: how many type pairs each level has kernels for +##### + +file(READ "${SVS_TYPE_PAIR_HEADER}" svs_pairs_text) +# Fold line continuations away so each #define is one line to match against. +string(REGEX REPLACE "\\\\[ \t]*\r?\n" " " svs_pairs_text "${svs_pairs_text}") + +# Returns the number of M(...) invocations in a type-pair list macro, following one +# level of aliasing -- `#define SVS_TYPE_PAIRS_AVX2 SVS_FOR_EACH_TYPE_PAIR`. +function(svs_type_pair_count out_var macro_name) + if(NOT svs_pairs_text MATCHES "#define ${macro_name}(\\([^)]*\\))?[ \t]+([^\n]*)") + message(FATAL_ERROR + "No `#define ${macro_name}` in ${SVS_TYPE_PAIR_HEADER}. Every ISA level " + "needs one; without it the level instantiates nothing." + ) + endif() + set(body "${CMAKE_MATCH_2}") + string(STRIP "${body}" body) + if(body MATCHES "^[A-Za-z_][A-Za-z0-9_]*$") + svs_type_pair_count(count "${body}") + set(${out_var} ${count} PARENT_SCOPE) + return() + endif() + string(REGEX MATCHALL "M\\(" hits "${body}") + list(LENGTH hits count) + if(count EQUAL 0) + message(FATAL_ERROR "${macro_name} in ${SVS_TYPE_PAIR_HEADER} invokes M zero times.") + endif() + set(${out_var} ${count} PARENT_SCOPE) +endfunction() + +##### +##### Ground truth 4: the distances +##### + +# Adding a distance means adding its Impl class here; otherwise its kernels are +# counted by nothing and a level could ship without them. +set(svs_distance_classes "L2Impl" "IPImpl" "CosineSimilarityImpl") + +set(svs_mangled_classes) +foreach(class IN LISTS svs_distance_classes) + string(LENGTH "${class}" len) + list(APPEND svs_mangled_classes "${len}${class}") +endforeach() + +##### +##### The expectation +##### + +# Keyed __; the count is the number of type pairs, +# one symbol each. +set(svs_expected_keys) +set(svs_expected_total 0) +foreach(level IN LISTS svs_expected_levels) + svs_type_pair_count(pairs "SVS_TYPE_PAIRS_${level}") + set(svs_pairs_of_${level} ${pairs}) + foreach(class IN LISTS svs_mangled_classes) + foreach(extent IN LISTS svs_expected_extents) + set(key "${class}_${extent}_${svs_digit_of_${level}}") + list(APPEND svs_expected_keys "${key}") + set(svs_want_${key} ${pairs}) + math(EXPR svs_expected_total "${svs_expected_total} + ${pairs}") + endforeach() + endforeach() +endforeach() + +##### +##### What was built +##### + +# Mangled names throughout: demangled ones carry `[clone .isra.0]` suffixes that +# differ between a local instantiation and an explicit one. +set(svs_kernel_regex "^_ZN3svs8distance([0-9]+[A-Za-z0-9_]+)ILm([0-9]+)E.*AVX_AVAILABILITYE([0-9]+)E") + +function(svs_symbols out_var file nm_arg) + execute_process( + COMMAND "${SVS_NM}" ${nm_arg} "${file}" + OUTPUT_VARIABLE raw + ERROR_VARIABLE err + RESULT_VARIABLE status + ) + if(NOT status EQUAL 0) + message(FATAL_ERROR "${SVS_NM} failed on ${file}: ${err}") + endif() + set(symbols) + string(REPLACE "\n" ";" lines "${raw}") + foreach(line IN LISTS lines) + if(line MATCHES "_ZN3svs8distance.*AVX_AVAILABILITY") + # 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) + set(${out_var} "${symbols}" PARENT_SCOPE) +endfunction() + +# Sets _KEYS, _FOREIGN and _HAVE_ in the caller. +function(svs_tally prefix symbols) + set(keys) + set(foreign) + foreach(symbol IN LISTS symbols) + if(NOT symbol MATCHES "${svs_kernel_regex}") + continue() + endif() + set(key "${CMAKE_MATCH_1}_${CMAKE_MATCH_2}_${CMAKE_MATCH_3}") + if(NOT key IN_LIST svs_expected_keys) + list(APPEND foreign "${symbol}") + continue() + endif() + if(NOT key IN_LIST keys) + list(APPEND keys "${key}") + set(${prefix}_HAVE_${key} 0) + endif() + math(EXPR next "${${prefix}_HAVE_${key}} + 1") + set(${prefix}_HAVE_${key} ${next}) + set(${prefix}_HAVE_${key} ${next} PARENT_SCOPE) + endforeach() + set(${prefix}_KEYS "${keys}" PARENT_SCOPE) + set(${prefix}_FOREIGN "${foreign}" PARENT_SCOPE) +endfunction() + +svs_symbols(archive_defines "${SVS_ARCHIVE}" --defined-only) +svs_symbols(consumer_defines "${SVS_CONSUMER_OBJECT}" --defined-only) +svs_symbols(consumer_references "${SVS_CONSUMER_OBJECT}" --undefined-only) + +svs_tally(ARCHIVE "${archive_defines}") +svs_tally(CONSUMER "${consumer_references}") + +##### +##### Compare +##### + +set(errors 0) + +function(svs_report_first list_var limit) + set(shown ${${list_var}}) + list(LENGTH shown count) + if(count GREATER limit) + list(SUBLIST shown 0 ${limit} shown) + endif() + foreach(entry IN LISTS shown) + message(" ${entry}") + endforeach() + if(count GREATER limit) + math(EXPR rest "${count} - ${limit}") + message(" ... and ${rest} more") + endif() +endfunction() + +# Sets _ERRORS in the caller. `what` names the thing being compared, for +# the failure message. +function(svs_compare prefix what) + set(wrong) + foreach(key IN LISTS svs_expected_keys) + set(have 0) + if(DEFINED ${prefix}_HAVE_${key}) + set(have ${${prefix}_HAVE_${key}}) + endif() + if(NOT have EQUAL ${svs_want_${key}}) + list(APPEND wrong "${key}: expected ${svs_want_${key}}, got ${have}") + endif() + endforeach() + list(LENGTH wrong n_wrong) + set(${prefix}_ERRORS 0 PARENT_SCOPE) + if(NOT n_wrong EQUAL 0) + message("${n_wrong} of ${what} disagree with ${SVS_SURFACE_FILE}.") + message("Each entry is __. A count of 0 means") + message("the declaration asks for kernels that were never built; a count") + message("below the number of type pairs means the level is missing some.") + message(" Mismatched:") + svs_report_first(wrong 10) + set(${prefix}_ERRORS 1 PARENT_SCOPE) + endif() +endfunction() + +svs_compare(ARCHIVE "the archive's kernel groups") +svs_compare(CONSUMER "the kernel groups a consumer reaches through the entry points") +math(EXPR errors "${ARCHIVE_ERRORS} + ${CONSUMER_ERRORS}") + +if(ARCHIVE_FOREIGN) + list(LENGTH ARCHIVE_FOREIGN n) + message("${n} kernels in the archive are outside the declared surface.") + message("Nothing in ${SVS_SURFACE_FILE} asks for them, so no consumer reaches") + message("them and they only add to the size of the library.") + message(" Outside the surface:") + svs_report_first(ARCHIVE_FOREIGN 10) + math(EXPR errors "${errors} + 1") +endif() + +# The whole point of the extern declarations. A kernel the consumer defines at a +# declared level came from the generic primary template at the consumer's own +# -march instead of from the library. +set(consumer_leaks) +foreach(symbol IN LISTS consumer_defines) + if(symbol MATCHES "${svs_kernel_regex}") + if(CMAKE_MATCH_3 IN_LIST svs_expected_digits) + list(APPEND consumer_leaks "${symbol}") + endif() + endif() +endforeach() +list(LENGTH consumer_leaks n_leaks) +if(NOT n_leaks EQUAL 0) + message("${n_leaks} kernels at a declared ISA level are instantiated by the consumer.") + message("Each is missing its `extern template` declaration, so every consumer of") + message("the headers builds it locally, from the generic scalar template, at") + message("whatever -march that consumer uses.") + message(" Instantiated locally:") + svs_report_first(consumer_leaks 10) + math(EXPR errors "${errors} + 1") +endif() + +if(NOT errors EQUAL 0) + message(FATAL_ERROR "dispatch declaration check failed") +endif() + +list(LENGTH svs_expected_levels n_levels) +list(LENGTH svs_expected_extents n_extents) +list(LENGTH svs_distance_classes n_distances) +set(pair_display) +foreach(level IN LISTS svs_expected_levels) + list(APPEND pair_display "${level}: ${svs_pairs_of_${level}}") +endforeach() +string(REPLACE ";" ", " pair_display "${pair_display}") +message( + "dispatch declaration: ${svs_expected_total} kernels required by " + "${SVS_SURFACE_FILE} (${n_levels} levels x ${n_extents} extents x " + "${n_distances} distances x type pairs per level {${pair_display}}); the " + "archive defines exactly those and the entry points reach all of them" +) diff --git a/cmake/check-dispatch-execution.cmake b/cmake/check-dispatch-execution.cmake new file mode 100644 index 000000000..7802fe9a9 --- /dev/null +++ b/cmake/check-dispatch-execution.cmake @@ -0,0 +1,155 @@ +# Copyright 2026 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 which ISA level a call through the entry points actually enters. +##### +##### Run in script mode: +##### +##### cmake -DSVS_PROBE= -DSVS_NM= -DSVS_GDB= \ +##### -P cmake/check-dispatch-execution.cmake +##### +##### Everything else about the surface is a property of the symbol table, which a +##### specialization can satisfy while never running: one that disappears behind an +##### `#if` still links and still counts. This breaks on every level's kernel for one +##### extent and reports the one the host's run reaches. +##### +##### Only the level this host satisfies is checked. The weaker levels are checked by +##### hosts that satisfy only those, which is what the CI matrix is for. +##### + +foreach(required SVS_PROBE SVS_NM SVS_GDB) + if(NOT ${required}) + message(FATAL_ERROR "${required} is not set.") + endif() +endforeach() +if(NOT EXISTS "${SVS_PROBE}") + message(FATAL_ERROR "SVS_PROBE does not exist: ${SVS_PROBE}") +endif() + +# The probe owns the runtime predicates, so it -- not this script -- decides which +# level the entry points are obliged to choose here. +execute_process( + COMMAND "${SVS_PROBE}" --report + OUTPUT_VARIABLE report + ERROR_VARIABLE err + RESULT_VARIABLE status +) +if(NOT status EQUAL 0) + message(FATAL_ERROR "${SVS_PROBE} --report failed: ${err}") +endif() +if(NOT report MATCHES "expect-level ([0-9]+)") + message(FATAL_ERROR "${SVS_PROBE} --report printed no expected level:\n${report}") +endif() +set(expected_digit "${CMAKE_MATCH_1}") +if(NOT report MATCHES "probe-extent ([0-9]+)") + message(FATAL_ERROR "${SVS_PROBE} --report printed no probe extent:\n${report}") +endif() +set(extent "${CMAKE_MATCH_1}") + +if(expected_digit EQUAL 0) + message( + "dispatch execution: this host satisfies no ISA level in the surface, so the " + "entry points can only reach AVX_AVAILABILITY::NONE; nothing to check" + ) + return() +endif() + +##### +##### The candidate symbols +##### + +# L2 at float/float (`ff`) for the extent the probe reports: one symbol per level, +# including any AVX_AVAILABILITY::NONE fallback the consumer instantiated itself. +execute_process( + COMMAND "${SVS_NM}" --defined-only "${SVS_PROBE}" + OUTPUT_VARIABLE raw + ERROR_VARIABLE err + RESULT_VARIABLE status +) +if(NOT status EQUAL 0) + message(FATAL_ERROR "${SVS_NM} failed on ${SVS_PROBE}: ${err}") +endif() + +set(candidates) +string(REPLACE "\n" ";" lines "${raw}") +foreach(line IN LISTS lines) + # The mangled name is the last whitespace-separated field. + string(REGEX MATCH "[^ \t]+$" symbol "${line}") + if(NOT symbol MATCHES "^_ZN3svs8distance6L2ImplILm${extent}Eff.*7computeE") + continue() + endif() + # Skip GCC's `.isra` clones: gdb reads a dot in a linespec as a file name. + if(symbol MATCHES "\\.") + continue() + endif() + list(APPEND candidates "${symbol}") +endforeach() +list(REMOVE_DUPLICATES candidates) + +list(LENGTH candidates n_candidates) +if(n_candidates EQUAL 0) + message(FATAL_ERROR + "No L2 kernel symbol for extent ${extent} at float/float is defined in " + "${SVS_PROBE}. Either the entry points inlined every kernel, in which case " + "the `extern template` declarations are not in effect, or the surface no " + "longer covers that extent." + ) +endif() + +##### +##### Run under the debugger +##### + +set(gdb_args -q -batch) +foreach(symbol IN LISTS candidates) + list(APPEND gdb_args -ex "break ${symbol}") +endforeach() +list(APPEND gdb_args -ex "run --report" -ex "info symbol $pc") + +execute_process( + COMMAND "${SVS_GDB}" ${gdb_args} "${SVS_PROBE}" + OUTPUT_VARIABLE gdb_out + ERROR_VARIABLE gdb_err + RESULT_VARIABLE status +) +if(NOT status EQUAL 0) + message("${gdb_out}") + message(FATAL_ERROR "${SVS_GDB} failed on ${SVS_PROBE}: ${gdb_err}") +endif() + +# gdb demangles for `info symbol`, so the level appears as the enum's underlying +# value: `(svs::distance::AVX_AVAILABILITY)2`. +if(NOT gdb_out MATCHES "AVX_AVAILABILITY\\)([0-9]+)") + message("${gdb_out}") + message(FATAL_ERROR + "The run never entered any of the ${n_candidates} kernels breakpointed for " + "extent ${extent}. A call through the entry points reached none of them, so " + "the dispatch is not routing to the surface." + ) +endif() +set(actual_digit "${CMAKE_MATCH_1}") + +if(NOT actual_digit EQUAL expected_digit) + message("The entry points routed a call to AVX_AVAILABILITY level ${actual_digit},") + message("but this host satisfies level ${expected_digit}. Either the runtime") + message("predicates in the entry points disagree with the ones the probe uses, or") + message("the enumerator order in the surface no longer matches the dispatch order.") + message(FATAL_ERROR "dispatch execution check failed") +endif() + +message( + "dispatch execution: a call to L2 at extent ${extent} entered " + "AVX_AVAILABILITY level ${actual_digit}, the highest this host satisfies" +) diff --git a/cmake/check-dispatch-instructions.cmake b/cmake/check-dispatch-instructions.cmake new file mode 100644 index 000000000..335b05f96 --- /dev/null +++ b/cmake/check-dispatch-instructions.cmake @@ -0,0 +1,148 @@ +# Copyright 2026 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 that an ISA level's object file stays inside its instruction budget. +##### +##### Run in script mode: +##### +##### cmake -DSVS_OBJECT= \ +##### -DSVS_LEVEL=AVX2 -DSVS_ARCH=haswell \ +##### -DSVS_OBJDUMP= \ +##### -P cmake/check-dispatch-instructions.cmake +##### +##### A level promises the host satisfies its runtime predicate and nothing more, +##### so an instruction the predicate does not guarantee is an illegal-instruction +##### fault on a host the dispatcher considers supported. +##### + +# Without it, CMP0007 is unset in script mode and the empty "forbids nothing" +# field of a budget row below is silently dropped rather than read as empty. +cmake_minimum_required(VERSION 3.21) + +foreach(required SVS_OBJECT SVS_LEVEL SVS_ARCH SVS_OBJDUMP) + if(NOT ${required}) + message(FATAL_ERROR "${required} is not set.") + endif() +endforeach() +if(NOT EXISTS "${SVS_OBJECT}") + message(FATAL_ERROR "SVS_OBJECT does not exist: ${SVS_OBJECT}") +endif() + +# What each instruction class looks like in AT&T disassembly. Register classes are +# matched with their `%` sigil so that a mangled name can never look like one. +set(svs_class_ymm "%ymm") +set(svs_class_zmm "%zmm") +set(svs_class_mask "%k[1-7]") +set(svs_class_vnni "vpdpwssd|vpdpbusd|vpdpwssds|vpdpbusds") + +# "<-march>||", one row per instruction budget +# any ISA level declares. A budget with no row here is a hard error: an +# unrecognized -march silently permitting everything is how a level ends up +# emitting instructions its runtime predicate does not guarantee. +set(svs_budget_table + "x86-64||ymm zmm mask vnni" + "haswell|ymm|zmm mask vnni" + "skylake-avx512|zmm|vnni" + "cascadelake|zmm|" +) + +set(svs_budget_found FALSE) +foreach(row IN LISTS svs_budget_table) + string(REPLACE "|" ";" fields "${row}") + list(GET fields 0 arch) + if(arch STREQUAL SVS_ARCH) + list(GET fields 1 svs_required) + list(GET fields 2 svs_forbidden) + set(svs_budget_found TRUE) + break() + endif() +endforeach() +if(NOT svs_budget_found) + message(FATAL_ERROR + "No instruction budget is known for -march=${SVS_ARCH} (ISA level " + "${SVS_LEVEL}). Add a row to svs_budget_table in this file stating what " + "that budget requires and forbids." + ) +endif() +string(REPLACE " " ";" svs_required "${svs_required}") +string(REPLACE " " ";" svs_forbidden "${svs_forbidden}") + +execute_process( + COMMAND "${SVS_OBJDUMP}" -d --no-show-raw-insn "${SVS_OBJECT}" + OUTPUT_VARIABLE svs_disassembly + ERROR_VARIABLE err + RESULT_VARIABLE status +) +if(NOT status EQUAL 0) + message(FATAL_ERROR "${SVS_OBJDUMP} failed on ${SVS_OBJECT}: ${err}") +endif() + +# Drop the `:` heading lines: they are the only place a mangled name +# appears, and a mangled name must not be mistaken for an instruction. +string(REGEX REPLACE "\n[0-9a-f]+ <[^\n]*>:" "\n" svs_disassembly "${svs_disassembly}") + +function(svs_count_class out_var class) + if(NOT DEFINED svs_class_${class}) + message(FATAL_ERROR "Unknown instruction class '${class}' in the budget table.") + endif() + string(REGEX MATCHALL "${svs_class_${class}}" hits "${svs_disassembly}") + list(LENGTH hits count) + set(${out_var} ${count} PARENT_SCOPE) +endfunction() + +set(errors 0) +set(summary) + +foreach(class IN LISTS svs_required) + svs_count_class(count "${class}") + if(count EQUAL 0) + message("${SVS_OBJECT} contains no ${class} instructions.") + message("ISA level ${SVS_LEVEL} is compiled at -march=${SVS_ARCH}, which is") + message("chosen precisely so the kernels use them. An object without any is") + message("a level built at the wrong budget, or one whose kernels fell back") + message("to the generic scalar template.") + math(EXPR errors "${errors} + 1") + else() + list(APPEND summary "${count} ${class}") + endif() +endforeach() + +foreach(class IN LISTS svs_forbidden) + svs_count_class(count "${class}") + if(NOT count EQUAL 0) + message("${SVS_OBJECT} contains ${count} ${class} instructions.") + message("ISA level ${SVS_LEVEL} guarantees only what its runtime predicate") + message("tests, so a host the dispatcher routes here faults on them. Either") + message("lower -march=${SVS_ARCH} in cmake/dispatch-surface.cmake, or give") + message("these kernels their own level with a predicate that covers them.") + math(EXPR errors "${errors} + 1") + endif() +endforeach() + +if(NOT errors EQUAL 0) + message(FATAL_ERROR "dispatch instruction check failed for level ${SVS_LEVEL}") +endif() + +string(REPLACE ";" ", " summary_display "${summary}") +if(svs_forbidden STREQUAL "") + set(forbidden_display "nothing forbidden") +else() + string(REPLACE ";" ", " forbidden_display "${svs_forbidden}") + set(forbidden_display "no ${forbidden_display}") +endif() +message( + "dispatch instructions: level ${SVS_LEVEL} at -march=${SVS_ARCH} has " + "${summary_display}, and ${forbidden_display}" +) diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake index a9bf2c385..65aaa7c43 100644 --- a/cmake/generate-dispatch-surface.cmake +++ b/cmake/generate-dispatch-surface.cmake @@ -145,6 +145,7 @@ endforeach() string(APPEND SVS_GEN_DIM_LOOP " /* end */") set(SVS_GEN_TARGET_LOOP "\\\n") +set(SVS_GEN_LEVEL_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) @@ -153,6 +154,7 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) list(GET level_fields 1 arch) list(GET level_fields 2 infix) + string(APPEND SVS_GEN_LEVEL_LOOP " M(${level}) \\\n") foreach(dim IN LISTS svs_dim_list) string(APPEND SVS_GEN_TARGET_LOOP " M(${dim}, ${level}) \\\n") endforeach() @@ -174,6 +176,7 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) ) endforeach() string(APPEND SVS_GEN_TARGET_LOOP " /* end */") +string(APPEND SVS_GEN_LEVEL_LOOP " /* end */") ##### ##### Emit the header diff --git a/cmake/templates/dispatch_surface.h.in b/cmake/templates/dispatch_surface.h.in index 01c1d5f17..64acd976d 100644 --- a/cmake/templates/dispatch_surface.h.in +++ b/cmake/templates/dispatch_surface.h.in @@ -33,4 +33,8 @@ // once per kernel the library compiles ahead of time, modulo type pairs. #define SVS_FOR_EACH_DISPATCH_TARGET(M) @SVS_GEN_TARGET_LOOP@ +// Invokes M(isa_level) once per ISA level, weakest first. AVX_AVAILABILITY +// enumerators without a translation unit are absent: this is the surface. +#define SVS_FOR_EACH_ISA_LEVEL(M) @SVS_GEN_LEVEL_LOOP@ + // clang-format on diff --git a/include/svs/core/distance/dispatch_surface.h b/include/svs/core/distance/dispatch_surface.h index 38bcc2853..a357ac0a8 100644 --- a/include/svs/core/distance/dispatch_surface.h +++ b/include/svs/core/distance/dispatch_surface.h @@ -62,4 +62,11 @@ M(svs::Dynamic, AVX512) \ /* end */ +// Invokes M(isa_level) once per ISA level, weakest first. AVX_AVAILABILITY +// enumerators without a translation unit are absent: this is the surface. +#define SVS_FOR_EACH_ISA_LEVEL(M) \ + M(AVX2) \ + M(AVX512) \ + /* end */ + // clang-format on diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index 20380b7f7..b192575f5 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -1,4 +1,4 @@ -# Copyright 2025 Intel Corporation +# Copyright 2026 Intel Corporation # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,45 +13,62 @@ # limitations under the License. ##### -##### The dispatch surface link probe. +##### Tests of the x86 dispatch surface. ##### -##### 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. +##### Not Catch2 tests: what is being checked is a property of an object file or of +##### a process, so each probe has to be its own translation unit, compiled at a +##### known instruction budget and inspected from outside. +##### +##### Not PROJECT_SOURCE_DIR anywhere below: the downstream repository adds this +##### directory to its own project, where that points somewhere else entirely. ##### -# 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 -) +set(svs_dispatch_cmake_dir "${CMAKE_CURRENT_LIST_DIR}/../../cmake") -# 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 -) +# The object libraries exist so that each probe's single object file can be named +# with $, which is what the symbol-table checks read. +# +# svs::x86_options_base is -march=x86-64 -mtune=generic: a probe must know no more +# about the host than an arbitrary consumer of the headers does. +function(svs_add_dispatch_probe name source) + add_library(${name}_objects OBJECT ${source}) + target_link_libraries( + ${name}_objects PRIVATE svs::svs svs::compile_options svs::x86_options_base + ) + add_executable(${name}) + target_link_libraries( + ${name} + PRIVATE ${name}_objects svs::svs svs::compile_options svs::x86_options_base + ) +endfunction() + +# Names every kernel the surface declares and nothing else, so a declared-but- +# uninstantiated kernel is an undefined symbol here. +svs_add_dispatch_probe(dispatch_surface_probe x86/link_probe.cpp) + +# Reaches the kernels through the entry points instead, so it also sees whether the +# entry points route to the whole surface. +svs_add_dispatch_probe(dispatch_entry_probe x86/entry_probe.cpp) # 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) +add_test(NAME dispatch_entry_probe COMMAND dispatch_entry_probe) if(DEFINED CMAKE_NM AND CMAKE_NM) set(svs_nm "${CMAKE_NM}") else() find_program(svs_nm NAMES nm llvm-nm) endif() +if(DEFINED CMAKE_OBJDUMP AND CMAKE_OBJDUMP) + set(svs_objdump "${CMAKE_OBJDUMP}") +else() + find_program(svs_objdump NAMES objdump llvm-objdump) +endif() +find_program(svs_gdb NAMES gdb) if(svs_nm) - # Host-independent, unlike the run above: it reads the symbol table rather + # Host-independent, unlike the runs 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 @@ -60,10 +77,64 @@ if(svs_nm) "-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" + -P "${svs_dispatch_cmake_dir}/check-dispatch-linkage.cmake" + ) + + # The same symbols, counted from the declaration instead of from a probe built + # out of the same generated header -- the one check a generator bug cannot pass + # by being wrong consistently. + add_test( + NAME dispatch_surface_declaration + COMMAND + "${CMAKE_COMMAND}" + "-DSVS_SURFACE_FILE=${SVS_DISPATCH_SURFACE_FILE}" + "-DSVS_TYPE_PAIR_HEADER=${CMAKE_CURRENT_LIST_DIR}/../../include/svs/multi-arch/x86/preprocessor.h" + "-DSVS_ENUM_HEADER=${CMAKE_CURRENT_LIST_DIR}/../../include/svs/core/distance/distance_core.h" + "-DSVS_ARCHIVE=$" + "-DSVS_CONSUMER_OBJECT=$" + "-DSVS_NM=${svs_nm}" + -P "${svs_dispatch_cmake_dir}/check-dispatch-declaration.cmake" + ) +else() + message(STATUS "nm not found; skipping the dispatch surface symbol-table tests") +endif() + +if(svs_objdump) + # One test per ISA level: a level whose object file carries instructions its + # runtime predicate does not guarantee faults on a host the dispatcher routes + # to it, which no symbol-table check can see. + foreach(tu_spec IN LISTS SVS_DISPATCH_TU_SPECS) + string(REPLACE "|" ";" tu_fields "${tu_spec}") + list(GET tu_fields 1 svs_level) + list(GET tu_fields 2 svs_arch) + list(GET tu_fields 3 svs_infix) + add_test( + NAME dispatch_instructions_${svs_infix} + COMMAND + "${CMAKE_COMMAND}" + "-DSVS_OBJECT=$" + "-DSVS_LEVEL=${svs_level}" + "-DSVS_ARCH=${svs_arch}" + "-DSVS_OBJDUMP=${svs_objdump}" + -P "${svs_dispatch_cmake_dir}/check-dispatch-instructions.cmake" + ) + endforeach() +else() + message(STATUS "objdump not found; skipping the dispatch surface instruction tests") +endif() + +if(svs_nm AND svs_gdb) + # The only check that observes a kernel run rather than exist: a specialization + # lost behind an `#if` still links and still counts. + add_test( + NAME dispatch_surface_execution + COMMAND + "${CMAKE_COMMAND}" + "-DSVS_PROBE=$" + "-DSVS_NM=${svs_nm}" + "-DSVS_GDB=${svs_gdb}" + -P "${svs_dispatch_cmake_dir}/check-dispatch-execution.cmake" ) else() - message(STATUS "nm not found; skipping the dispatch surface linkage test") + message(STATUS "gdb not found; skipping the dispatch surface execution test") endif() diff --git a/tests/multi-arch/x86/entry_probe.cpp b/tests/multi-arch/x86/entry_probe.cpp new file mode 100644 index 000000000..55d87052e --- /dev/null +++ b/tests/multi-arch/x86/entry_probe.cpp @@ -0,0 +1,112 @@ +/* + * Copyright 2026 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 reaches the kernels the way real code does: through the +// distance entry points, which name every ISA level unconditionally and pick one +// at runtime. Complements link_probe.cpp, which names the kernels directly and so +// checks the surface without depending on the entry points routing to it. + +#include "svs/core/distance/cosine.h" +#include "svs/core/distance/euclidean.h" +#include "svs/core/distance/inner_product.h" +#include "svs/multi-arch/x86/preprocessor.h" + +#include "host_levels.h" + +#include +#include +#include +#include +#include + +namespace { + +// Deliberately not a multiple of any vector width, so the epilogue is exercised. +constexpr size_t entry_dynamic_dim = 97; + +constexpr size_t fixed_extent(bool longest) { + size_t result = longest ? 1 : svs::Dynamic; + for (auto dim : svs::distance::supported_dim_list) { + if (dim == svs::Dynamic) { + continue; + } + result = longest ? std::max(result, dim) : std::min(result, dim); + } + return result; +} + +// The extent the execution check breaks on: the surface's shortest fixed one, so +// the kernel it enters is a fully unrolled specialization rather than the epilogue. +constexpr size_t entry_report_dim = fixed_extent(false); + +constexpr size_t entry_buffer_dim = std::max(fixed_extent(true), entry_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(); +} + +// A template rather than a macro body: the svs::Dynamic entry points take a +// length, and `if constexpr` only discards the other branch inside a template. +template float entry_one() { + const Ea* a = buffer(); + const Eb* b = buffer(); + if constexpr (N == svs::Dynamic) { + return svs::distance::L2::compute(a, b, entry_dynamic_dim) + + svs::distance::IP::compute(a, b, entry_dynamic_dim) + + svs::distance::CosineSimilarity::compute(a, b, 1.0F, entry_dynamic_dim); + } else { + return svs::distance::L2::compute(a, b) + svs::distance::IP::compute(a, b) + + svs::distance::CosineSimilarity::compute(a, b, 1.0F); + } +} + +#define SVS_ENTRY_ONE(Ea, Eb, N) total += entry_one(); +#define SVS_ENTRY_DIM(N) SVS_FOR_EACH_TYPE_PAIR(SVS_ENTRY_ONE, N) + +float entry_all() { + float total = 0; + SVS_FOR_EACH_SUPPORTED_DIM(SVS_ENTRY_DIM) + return total; +} + +#undef SVS_ENTRY_DIM +#undef SVS_ENTRY_ONE + +} // namespace + +int main(int argc, char** argv) { + if (argc == 2 && std::strcmp(argv[1], "--report") == 0) { + // Read by cmake/check-dispatch-execution.cmake, which then breaks on the + // kernels of this extent and checks which level the call below enters. + std::printf("expect-level %d\n", svs_test::expected_level()); + std::printf("probe-extent %zu\n", entry_report_dim); + std::printf( + "one-call %f\n", + static_cast(entry_one()) + ); + return 0; + } + + // Printing keeps the calls from being optimized away, and makes the run a + // smoke test of every entry point over the whole surface. + std::printf("dispatch surface entry probe: %f\n", static_cast(entry_all())); + return 0; +} diff --git a/tests/multi-arch/x86/host_levels.h b/tests/multi-arch/x86/host_levels.h new file mode 100644 index 000000000..a94c42973 --- /dev/null +++ b/tests/multi-arch/x86/host_levels.h @@ -0,0 +1,56 @@ +/* + * Copyright 2026 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. + */ + +#pragma once + +#include "svs/core/distance/distance_core.h" +#include "svs/lib/avx_detection.h" + +#include + +namespace svs_test { + +using svs::distance::AVX_AVAILABILITY; + +// The runtime predicate behind each ISA level, and the only place the dispatch +// tests state it: a level added to the surface without a specialization here is +// an undefined symbol, deliberately -- the predicate cannot be guessed. +template bool host_satisfies(); + +template <> inline bool host_satisfies() { + return svs::detail::avx_runtime_flags.is_avx2_supported(); +} + +template <> inline bool host_satisfies() { + return svs::detail::avx_runtime_flags.is_avx512f_supported(); +} + +/// The level the distance entry points must choose on this host. +/// +/// The entry points test the strongest level first, so their choice is the +/// highest-numbered satisfied level; AVX_AVAILABILITY::NONE is the fallback. +inline int expected_level() { + int highest = static_cast(AVX_AVAILABILITY::NONE); +#define SVS_HOST_LEVEL_ONE(LEVEL) \ + if (host_satisfies()) { \ + highest = std::max(highest, static_cast(AVX_AVAILABILITY::LEVEL)); \ + } + SVS_FOR_EACH_ISA_LEVEL(SVS_HOST_LEVEL_ONE) +#undef SVS_HOST_LEVEL_ONE + return highest; +} + +} // namespace svs_test diff --git a/tests/multi-arch/x86/link_probe.cpp b/tests/multi-arch/x86/link_probe.cpp index d7a82a288..443c87e62 100644 --- a/tests/multi-arch/x86/link_probe.cpp +++ b/tests/multi-arch/x86/link_probe.cpp @@ -20,10 +20,11 @@ #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 "host_levels.h" + #include #include #include @@ -32,18 +33,7 @@ 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(); -} +using svs_test::host_satisfies; // The longest fixed extent in the surface. constexpr size_t probe_max_dim = []() { From 5fafe1da1c9109b7023a591c154a4bb24272c36b Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 24 Aug 2026 09:02:49 -0700 Subject: [PATCH 06/23] docs(dispatch): document the surface bookkeeping and group the checkers Answer the review on the declaration's maintenance story: cmake/dispatch-surface.cmake now states what to edit for an extent, a level, a type pair or an instruction budget, and why AVX_AVAILABILITY::NONE has no row. Move the four checker scripts to cmake/dispatch-checks/ with a README, and make the preprocessor.h type-pair comment stand without the refactoring for context. Co-Authored-By: Claude Opus 5 --- cmake/AGENTS.md | 1 + cmake/dispatch-checks/README.md | 66 +++++++++++++++++++ .../check-dispatch-declaration.cmake | 15 +++-- .../check-dispatch-execution.cmake | 2 +- .../check-dispatch-instructions.cmake | 2 +- .../check-dispatch-linkage.cmake | 2 +- cmake/dispatch-surface.cmake | 35 ++++++++++ include/svs/multi-arch/x86/preprocessor.h | 4 +- tests/multi-arch/CMakeLists.txt | 2 +- tests/multi-arch/x86/entry_probe.cpp | 4 +- 10 files changed, 118 insertions(+), 15 deletions(-) create mode 100644 cmake/dispatch-checks/README.md rename cmake/{ => dispatch-checks}/check-dispatch-declaration.cmake (95%) rename cmake/{ => dispatch-checks}/check-dispatch-execution.cmake (98%) rename cmake/{ => dispatch-checks}/check-dispatch-instructions.cmake (98%) rename cmake/{ => dispatch-checks}/check-dispatch-linkage.cmake (98%) diff --git a/cmake/AGENTS.md b/cmake/AGENTS.md index f6f81aecb..8b6da30b9 100644 --- a/cmake/AGENTS.md +++ b/cmake/AGENTS.md @@ -10,6 +10,7 @@ Build modules, dependency wiring, and feature toggles. ## Intel-specific modules - **`cmake/mkl.cmake`:** MKL linkage (static vs dynamic threading). Do not hardcode MKL versions. When changing linkage mode, validate threading behavior in tests. - **`cmake/multi-arch.cmake`:** AVX-512 / SIMD ISA dispatch. Do not hardcode `-march` or ISA flags outside this file. Changes must align with `include/svs/multi-arch/` runtime dispatch code. +- **`cmake/dispatch-surface.cmake`:** The declared x86 dispatch surface — the fixed extents and the ISA levels. Edit it, never the generated `include/svs/core/distance/dispatch_surface.h`, which every configure overwrites. `cmake/dispatch-checks/` holds the ctest checkers that hold the built binary to this declaration. - **`cmake/numa.cmake`:** NUMA-aware memory allocation. Respect NUMA topology assumptions in performance-critical code. - **`cmake/openmp.cmake`:** Threading model. Do not assume specific OpenMP version or runtime without checking source-of-truth. diff --git a/cmake/dispatch-checks/README.md b/cmake/dispatch-checks/README.md new file mode 100644 index 000000000..331e913f6 --- /dev/null +++ b/cmake/dispatch-checks/README.md @@ -0,0 +1,66 @@ + + +# Dispatch surface checks + +These scripts are run by ctest to hold the built binary to the surface +declared in `cmake/dispatch-surface.cmake`. They are not part of building +the library. See `cmake/dispatch-surface.cmake` for documentation on the +declaration format, the current extent list and ISA levels, and how to add +a new level. + +These tests are only added to the build when the x86 object libraries +exist. Run `ctest` from `/tests`, not from the build root. + +## check-dispatch-linkage.cmake + +**Test:** `dispatch_surface_linkage` + +Verifies that every kernel declared in the surface is defined in the +archive, that the archive defines no undeclared kernels, and that a test +probe naming every kernel defines none of its own. Failure means a +declared kernel is missing or an extra kernel was built but not declared. + +## check-dispatch-declaration.cmake + +**Test:** `dispatch_surface_declaration` + +Verifies that the symbol count derived from the surface declaration matches +the symbol count in the archive and in a consumer object that names every +entry point. Failure means the declaration's extent list, ISA levels, type +pairs, or distance enumerator order disagrees with the built kernels. This +catches generator bugs that would pass linkage checks by being wrong +consistently in both the archive and the probe. + +## check-dispatch-instructions.cmake + +**Test:** `dispatch_instructions_` (one test per ISA level) + +Verifies that each level's object file stays within its instruction budget, +emitting only instructions the level's runtime predicate guarantees the host +supports. Failure means an object file contains instructions not guaranteed +by its level's predicate, causing illegal-instruction faults on hosts the +dispatcher routes to that level. + +## check-dispatch-execution.cmake + +**Test:** `dispatch_surface_execution` + +Verifies that a call through the entry points on the test host enters the +ISA level the host satisfies, observed by breaking in gdb on each level's +kernel and reporting which one runs. Failure means runtime dispatch routes +to the wrong level, or a specialization disappeared behind a preprocessor +guard while still linking and counting in the symbol table. diff --git a/cmake/check-dispatch-declaration.cmake b/cmake/dispatch-checks/check-dispatch-declaration.cmake similarity index 95% rename from cmake/check-dispatch-declaration.cmake rename to cmake/dispatch-checks/check-dispatch-declaration.cmake index 20d5c4c1d..4e50d83f7 100644 --- a/cmake/check-dispatch-declaration.cmake +++ b/cmake/dispatch-checks/check-dispatch-declaration.cmake @@ -24,14 +24,15 @@ ##### -DSVS_ARCHIVE= \ ##### -DSVS_CONSUMER_OBJECT= \ ##### -DSVS_NM= \ -##### -P cmake/check-dispatch-declaration.cmake +##### -P cmake/dispatch-checks/check-dispatch-declaration.cmake ##### -##### cmake/check-dispatch-linkage.cmake compares the archive against a probe, and -##### both are generated from the same header: a generator that dropped an extent -##### would drop it from both and still agree. The expectation here is derived from -##### the three hand-written sources instead -- the extent list and levels, the type -##### pairs, and the enumerator order -- so the generated header is not consulted at -##### all and a generator bug has nowhere to hide. +##### cmake/dispatch-checks/check-dispatch-linkage.cmake compares the archive +##### against a probe, and both are generated from the same header: a generator +##### that dropped an extent would drop it from both and still agree. The +##### expectation here is derived from the three hand-written sources instead -- +##### the extent list and levels, the type pairs, and the enumerator order -- so +##### the generated header is not consulted at all and a generator bug has nowhere +##### to hide. ##### # Without it, CMP0057 is unset in script mode and the `IN_LIST` tests below are diff --git a/cmake/check-dispatch-execution.cmake b/cmake/dispatch-checks/check-dispatch-execution.cmake similarity index 98% rename from cmake/check-dispatch-execution.cmake rename to cmake/dispatch-checks/check-dispatch-execution.cmake index 7802fe9a9..e15eb9f2a 100644 --- a/cmake/check-dispatch-execution.cmake +++ b/cmake/dispatch-checks/check-dispatch-execution.cmake @@ -18,7 +18,7 @@ ##### Run in script mode: ##### ##### cmake -DSVS_PROBE= -DSVS_NM= -DSVS_GDB= \ -##### -P cmake/check-dispatch-execution.cmake +##### -P cmake/dispatch-checks/check-dispatch-execution.cmake ##### ##### Everything else about the surface is a property of the symbol table, which a ##### specialization can satisfy while never running: one that disappears behind an diff --git a/cmake/check-dispatch-instructions.cmake b/cmake/dispatch-checks/check-dispatch-instructions.cmake similarity index 98% rename from cmake/check-dispatch-instructions.cmake rename to cmake/dispatch-checks/check-dispatch-instructions.cmake index 335b05f96..fafd97d2d 100644 --- a/cmake/check-dispatch-instructions.cmake +++ b/cmake/dispatch-checks/check-dispatch-instructions.cmake @@ -20,7 +20,7 @@ ##### cmake -DSVS_OBJECT= \ ##### -DSVS_LEVEL=AVX2 -DSVS_ARCH=haswell \ ##### -DSVS_OBJDUMP= \ -##### -P cmake/check-dispatch-instructions.cmake +##### -P cmake/dispatch-checks/check-dispatch-instructions.cmake ##### ##### A level promises the host satisfies its runtime predicate and nothing more, ##### so an instruction the predicate does not guarantee is an illegal-instruction diff --git a/cmake/check-dispatch-linkage.cmake b/cmake/dispatch-checks/check-dispatch-linkage.cmake similarity index 98% rename from cmake/check-dispatch-linkage.cmake rename to cmake/dispatch-checks/check-dispatch-linkage.cmake index c8d542561..6ec22b711 100644 --- a/cmake/check-dispatch-linkage.cmake +++ b/cmake/dispatch-checks/check-dispatch-linkage.cmake @@ -20,7 +20,7 @@ ##### cmake -DSVS_PROBE_OBJECT= \ ##### -DSVS_ARCHIVE= \ ##### -DSVS_NM= \ -##### -P cmake/check-dispatch-linkage.cmake +##### -P cmake/dispatch-checks/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* diff --git a/cmake/dispatch-surface.cmake b/cmake/dispatch-surface.cmake index 341c89079..971b1785f 100644 --- a/cmake/dispatch-surface.cmake +++ b/cmake/dispatch-surface.cmake @@ -30,6 +30,32 @@ ##### in which case the committed header is left alone and only the build tree ##### describes that surface. ##### +##### Bookkeeping +##### +##### To add or remove a fixed extent: +##### Edit `SVS_SUPPORTED_DIMS`. The generated header, every `extern +##### template`, and `supported_dim_list` follow automatically. +##### +##### To add an ISA level: +##### 1. Add a row to `SVS_ISA_LEVELS`. +##### 2. Add a `SVS_TYPE_PAIRS_` list in +##### include/svs/multi-arch/x86/preprocessor.h, listing the element-type +##### pairs that level has kernels for. +##### 3. Add the level's translation unit at +##### include/svs/multi-arch/x86/.cpp. +##### The object library and its compile flags follow from the row here. A +##### level without a type-pair list is a compile error, not an empty +##### instantiation set. +##### +##### To add or remove an element-type pair for a level: +##### Edit that level's `SVS_TYPE_PAIRS_` list in +##### include/svs/multi-arch/x86/preprocessor.h. Not configured here: a type +##### pair exists because an implementation exists for it. +##### +##### To change a level's instruction budget: +##### Edit the middle field of its row in `SVS_ISA_LEVELS`, observing the +##### constraint recorded there. +##### # Extents that get their own fixed-extent kernel. # @@ -45,6 +71,15 @@ set(SVS_SUPPORTED_DIMS 64 96 100 128 160 200 512 768) # Runtime ISA levels. Each has one translation unit, which instantiates every # extent above at that level. # +# Only levels with their own translation unit appear here, which is why +# `AVX_AVAILABILITY::NONE` has no row: nothing instantiates its kernels ahead +# of time, so each translation unit that uses them instantiates them itself, at +# its own `-march` -- generic `x86-64` for this project's own build. A row here +# would name a translation unit and an object library that do not exist. +# +# `AVX_AVAILABILITY` mangles positionally, so `NONE` must keep its enumerator +# value despite having no row; renumbering the enumerators is an ABI break. +# # || # # enumerator a value of `svs::distance::AVX_AVAILABILITY` diff --git a/include/svs/multi-arch/x86/preprocessor.h b/include/svs/multi-arch/x86/preprocessor.h index 2f3f71af7..75941b70f 100644 --- a/include/svs/multi-arch/x86/preprocessor.h +++ b/include/svs/multi-arch/x86/preprocessor.h @@ -21,8 +21,8 @@ ///// ///// Element-type pairs. ///// -///// Hand-written, unlike the generated extent list: a pair is here because a -///// kernel exists for it, and generating it would invite unimplemented pairs. +///// Every (query, dataset) combination of float, int8_t, uint8_t and Float16. +///// Each pair costs one instantiation per extent, ISA level and distance. ///// // Invokes M(query_type, dataset_type, ...) once per type pair. diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index b192575f5..d52c1bc2e 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -23,7 +23,7 @@ ##### directory to its own project, where that points somewhere else entirely. ##### -set(svs_dispatch_cmake_dir "${CMAKE_CURRENT_LIST_DIR}/../../cmake") +set(svs_dispatch_cmake_dir "${CMAKE_CURRENT_LIST_DIR}/../../cmake/dispatch-checks") # The object libraries exist so that each probe's single object file can be named # with $, which is what the symbol-table checks read. diff --git a/tests/multi-arch/x86/entry_probe.cpp b/tests/multi-arch/x86/entry_probe.cpp index 55d87052e..567ae6784 100644 --- a/tests/multi-arch/x86/entry_probe.cpp +++ b/tests/multi-arch/x86/entry_probe.cpp @@ -94,8 +94,8 @@ float entry_all() { int main(int argc, char** argv) { if (argc == 2 && std::strcmp(argv[1], "--report") == 0) { - // Read by cmake/check-dispatch-execution.cmake, which then breaks on the - // kernels of this extent and checks which level the call below enters. + // Read by cmake/dispatch-checks/check-dispatch-execution.cmake, which + // breaks on this extent's kernels to see which level the call enters. std::printf("expect-level %d\n", svs_test::expected_level()); std::printf("probe-extent %zu\n", entry_report_dim); std::printf( From a715124991786db6071bbbd1a41109c0d396bc54 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Aug 2026 07:09:40 -0700 Subject: [PATCH 07/23] feat(dispatch): Validate the dispatch-surface declaration at configure time 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 twelve that must be rejected, each carrying the substring its rejection has to mention. `.github/scripts/check_dispatch_surface.sh` runs the lot -- fifteen cases, counting the default declaration -- in a fraction of a second, needing no compiler and no build tree. 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 fixed 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 | 18 +++ cmake/generate-dispatch-surface.cmake | 118 ++------------ 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, 664 insertions(+), 102 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..6583ed6ef --- /dev/null +++ b/.github/scripts/check_dispatch_surface.sh @@ -0,0 +1,111 @@ +#!/bin/bash +# Copyright 2026 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..df6335240 --- /dev/null +++ b/.github/workflows/dispatch-surface.yml @@ -0,0 +1,106 @@ +# Copyright 2026 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..df9502bd3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,3 +34,21 @@ 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 + # (?x) ignores the layout below; without it the alternation is one + # unreadable line and every edit to it risks silently matching nothing. + files: | + (?x)^( + 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 65aaa7c43..b1aa9a7c6 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,92 +53,15 @@ 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 */") @@ -147,7 +69,6 @@ string(APPEND SVS_GEN_DIM_LOOP " /* end */") set(SVS_GEN_TARGET_LOOP "\\\n") set(SVS_GEN_LEVEL_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) @@ -155,22 +76,15 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) list(GET level_fields 2 infix) string(APPEND SVS_GEN_LEVEL_LOOP " M(${level}) \\\n") - 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}") + # One translation unit per level, committed rather than generated because the + # downstream repository compiles these sources by path. Validation checks it exists. + list(APPEND SVS_DISPATCH_TU_SPECS + "${SVS_X86_SRC_DIR}/${infix}.cpp|${level}|${arch}|${infix}" + ) list(APPEND svs_level_report "AVX_AVAILABILITY::${level} -march=${arch} ${infix}.cpp" ) @@ -217,7 +131,7 @@ endif() list(LENGTH SVS_ISA_LEVELS svs_level_count) string(REPLACE ";" " " svs_dims_display "${SVS_SUPPORTED_DIMS}") message(STATUS - "Dispatch surface: ${SVS_GEN_DIM_COUNT} extents x ${svs_level_count} ISA levels" + "Dispatch surface: ${SVS_DIM_COUNT} extents x ${svs_level_count} ISA levels" ) message(STATUS " extents: ${svs_dims_display} svs::Dynamic") foreach(entry IN LISTS svs_level_report) diff --git a/cmake/validate-dispatch-surface.cmake b/cmake/validate-dispatch-surface.cmake new file mode 100644 index 000000000..7bf1f21a6 --- /dev/null +++ b/cmake/validate-dispatch-surface.cmake @@ -0,0 +1,148 @@ +# Copyright 2026 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..fe809c05d --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-extent.cmake @@ -0,0 +1,17 @@ +# Copyright 2026 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..8e05c06fe --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-infix.cmake @@ -0,0 +1,22 @@ +# Copyright 2026 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..091109839 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-level.cmake @@ -0,0 +1,21 @@ +# Copyright 2026 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..127527fcf --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-dynamic-listed.cmake @@ -0,0 +1,18 @@ +# Copyright 2026 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..5f548ba90 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-level-empty-field.cmake @@ -0,0 +1,17 @@ +# Copyright 2026 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..2f71f45f6 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake @@ -0,0 +1,18 @@ +# Copyright 2026 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..3ffc6e495 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-level-without-tu.cmake @@ -0,0 +1,22 @@ +# Copyright 2026 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..b5cfb55a8 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-missing-extents.cmake @@ -0,0 +1,17 @@ +# Copyright 2026 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..d303cde71 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-no-extents.cmake @@ -0,0 +1,17 @@ +# Copyright 2026 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..c3c2b4a98 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-no-levels.cmake @@ -0,0 +1,17 @@ +# Copyright 2026 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..2af432451 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-non-numeric-extent.cmake @@ -0,0 +1,17 @@ +# Copyright 2026 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..c74f51e90 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-zero-extent.cmake @@ -0,0 +1,18 @@ +# Copyright 2026 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..089a50eef --- /dev/null +++ b/tests/cmake/dispatch-surface/valid-minimal.cmake @@ -0,0 +1,19 @@ +# Copyright 2026 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..4a9bfcc04 --- /dev/null +++ b/tests/cmake/dispatch-surface/valid-reduced.cmake @@ -0,0 +1,25 @@ +# Copyright 2026 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 23cd4cc50ed70f8efb21ebefc0d8b28054ebf4b0 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Aug 2026 11:42:56 -0700 Subject: [PATCH 08/23] Make VNNI its own ISA level, and lower the AVX512 level to skylake-avx512 The AVX512 level's translation unit was compiled at -march=cascadelake, which enables AVX512-VNNI. That level promises AVX-512 F/BW/DQ about the host and nothing more, so every kernel in that object file was compiled with permission to use instructions a Skylake-SP does not have. The VNNI kernels that existed guarded themselves with a runtime check, but the guard only covered the calls that were written by hand; the compiler was free to emit vpdpwssd anywhere in the TU on its own initiative. Adding AVX_AVAILABILITY::AVX512_VNNI as a fourth level moves the check to the one place a level is chosen -- the entry point -- and lets each TU be compiled at exactly what its level promises. The int8/int8 and uint8/uint8 kernels move to the new level; every other pair promotes to float before doing arithmetic, where VNNI has nothing to offer, so those pairs have no kernel at this level. That is what keeps a fourth level from costing a fourth of everything: 54 new instantiations rather than 432. Two consequences worth naming: - The pairs that move need an AVX512-level kernel to fall back to, and it has to live outside `#if SVS_AVX512_VNNI`. Inside, it would be absent from the AVX512 TU -- which is now compiled where that macro is 0 -- and silently replaced by the generic template. This is why the two halves of the change cannot land separately. - The entry points must not dispatch to a level that has no kernel for the pair in hand, for the same reason. `svs::distance::has_vnni_kernel` answers that, generated from the same list the kernels are, and it is `if constexpr` so it compiles away for the pairs that do not move. The generated header now also defines SVS_ISA_LEVEL_ per level, so a surface that leaves a level out is visible to the code that dispatches on it. Dropping the VNNI level degrades correctly -- those pairs stay on AVX512, and the probe reports 864 kernels instead of 918. Dropping AVX2 or AVX512 is an `#error` instead, because the entry points reach those two for every type pair. ISA levels are not configuration the way the extent list is: a level exists because kernels, a TU and a CPUID check exist for it. The dispatch checks pick the change up on their own, which is what they were written for. dispatch_instructions_avx512 now judges avx512.cpp.o at skylake-avx512 and so forbids VNNI there, and it fails on the old object file; the cascadelake row gains `vnni` as a requirement, because a VNNI level whose object file has no VNNI in it is 54 instantiations of dead weight. Three mechanical follow-ons: the per-level object libraries are named after the level rather than the -march, since two levels now share neither; the execution check breaks on int8/int8 rather than float/float, as a float-promoting pair has no kernel at the top level and would route one lower; and the VNNI predicate joins the other two in tests/multi-arch/x86/host_levels.h. Verified on the default surface: 918 kernels declared, instantiated and reachable with none instantiated by the consumer; all 456 vpdpwssd encodings in vnni.cpp.o, zero in avx512.cpp.o and avx2.cpp.o, where before all 456 sat in the AVX512 level's object file; the AVX2 object identical in symbol names and sizes to before; the new L2Impl<128,int8,int8,AVX512> vectorized 16-wide float, not scalar. `[distance]` passes with the same 134402115 assertions as before, all eight dispatch tests pass, and ctest is otherwise unchanged. Also verified with SVS_NO_AVX512=YES (both TUs compile generic, zero AVX-512 encodings, all 918 still linked) and with the reduced surface (306 kernels). Co-Authored-By: Claude Opus 5 --- .../check-dispatch-execution.cmake | 20 ++- .../check-dispatch-instructions.cmake | 2 +- cmake/dispatch-surface.cmake | 28 +++- cmake/generate-dispatch-surface.cmake | 4 + cmake/multi-arch.cmake | 4 +- cmake/templates/dispatch_surface.h.in | 4 + include/svs/core/distance/cosine.h | 140 +++++++++++------- include/svs/core/distance/dispatch_surface.h | 16 ++ include/svs/core/distance/distance_core.h | 31 +++- include/svs/core/distance/euclidean.h | 58 ++++++-- include/svs/core/distance/inner_product.h | 58 ++++++-- include/svs/multi-arch/x86/preprocessor.h | 6 + include/svs/multi-arch/x86/vnni.cpp | 32 ++++ .../dispatch-surface/valid-reduced.cmake | 11 +- tests/multi-arch/CMakeLists.txt | 2 +- tests/multi-arch/x86/entry_probe.cpp | 5 +- tests/multi-arch/x86/host_levels.h | 4 + tests/multi-arch/x86/link_probe.cpp | 8 +- 18 files changed, 334 insertions(+), 99 deletions(-) create mode 100644 include/svs/multi-arch/x86/vnni.cpp diff --git a/cmake/dispatch-checks/check-dispatch-execution.cmake b/cmake/dispatch-checks/check-dispatch-execution.cmake index e15eb9f2a..f16ff3a20 100644 --- a/cmake/dispatch-checks/check-dispatch-execution.cmake +++ b/cmake/dispatch-checks/check-dispatch-execution.cmake @@ -23,7 +23,7 @@ ##### Everything else about the surface is a property of the symbol table, which a ##### specialization can satisfy while never running: one that disappears behind an ##### `#if` still links and still counts. This breaks on every level's kernel for one -##### extent and reports the one the host's run reaches. +##### extent and type pair, and reports the one the host's run reaches. ##### ##### Only the level this host satisfies is checked. The weaker levels are checked by ##### hosts that satisfy only those, which is what the CI matrix is for. @@ -70,8 +70,12 @@ endif() ##### The candidate symbols ##### -# L2 at float/float (`ff`) for the extent the probe reports: one symbol per level, -# including any AVX_AVAILABILITY::NONE fallback the consumer instantiated itself. +# The type pair the probe calls, Itanium-mangled. It must be one with a kernel at +# every level in the surface, or the entry points legitimately route lower. +set(svs_probe_pair "aa") + +# L2 at that pair for the extent the probe reports: one symbol per level, including +# any AVX_AVAILABILITY::NONE fallback the consumer instantiated itself. execute_process( COMMAND "${SVS_NM}" --defined-only "${SVS_PROBE}" OUTPUT_VARIABLE raw @@ -87,7 +91,7 @@ string(REPLACE "\n" ";" lines "${raw}") foreach(line IN LISTS lines) # The mangled name is the last whitespace-separated field. string(REGEX MATCH "[^ \t]+$" symbol "${line}") - if(NOT symbol MATCHES "^_ZN3svs8distance6L2ImplILm${extent}Eff.*7computeE") + if(NOT symbol MATCHES "^_ZN3svs8distance6L2ImplILm${extent}E${svs_probe_pair}.*7computeE") continue() endif() # Skip GCC's `.isra` clones: gdb reads a dot in a linespec as a file name. @@ -101,10 +105,10 @@ list(REMOVE_DUPLICATES candidates) list(LENGTH candidates n_candidates) if(n_candidates EQUAL 0) message(FATAL_ERROR - "No L2 kernel symbol for extent ${extent} at float/float is defined in " - "${SVS_PROBE}. Either the entry points inlined every kernel, in which case " - "the `extern template` declarations are not in effect, or the surface no " - "longer covers that extent." + "No L2 kernel symbol for extent ${extent} at the mangled type pair " + "'${svs_probe_pair}' is defined in ${SVS_PROBE}. Either the entry points " + "inlined every kernel, in which case the `extern template` declarations are " + "not in effect, or the probe no longer calls that extent and pair." ) endif() diff --git a/cmake/dispatch-checks/check-dispatch-instructions.cmake b/cmake/dispatch-checks/check-dispatch-instructions.cmake index fafd97d2d..45e1f0698 100644 --- a/cmake/dispatch-checks/check-dispatch-instructions.cmake +++ b/cmake/dispatch-checks/check-dispatch-instructions.cmake @@ -55,7 +55,7 @@ set(svs_budget_table "x86-64||ymm zmm mask vnni" "haswell|ymm|zmm mask vnni" "skylake-avx512|zmm|vnni" - "cascadelake|zmm|" + "cascadelake|zmm vnni|" ) set(svs_budget_found FALSE) diff --git a/cmake/dispatch-surface.cmake b/cmake/dispatch-surface.cmake index 971b1785f..7cbe19b22 100644 --- a/cmake/dispatch-surface.cmake +++ b/cmake/dispatch-surface.cmake @@ -91,11 +91,29 @@ set(SVS_SUPPORTED_DIMS 64 96 100 128 160 200 512 768) # include/svs/multi-arch/x86/.cpp, and the object # library it is compiled into # -# Adding a level here also requires a `SVS_TYPE_PAIRS_` list in -# include/svs/multi-arch/x86/preprocessor.h, saying which element-type pairs -# that level has kernels for. That is deliberately not configured here: a type -# pair exists because an implementation exists for it. +# Unlike the extent list above, this list is not a knob. A level exists because +# kernels, a translation unit and a runtime check for it exist, so changing it +# means changing code: +# +# - a `SVS_TYPE_PAIRS_` list in +# include/svs/multi-arch/x86/preprocessor.h, saying which element-type pairs +# the level has kernels for +# - the specializations themselves, in the three distance headers +# - a branch in the entry points, and the CPUID check it tests +# +# The generated header defines `SVS_ISA_LEVEL_` for each level listed +# here, which is how the entry points tell a level that is present from one that +# is not; dropping AVX2 or AVX512 from this list is a compile error rather than a +# silent fall back to unvectorized code. +# +# On the instruction budgets in particular: `skylake-avx512` rather than +# `cascadelake` for the AVX512 level, because that level promises AVX-512 F/BW/DQ +# and nothing more. `cascadelake` also enables AVX512-VNNI, which the compiler is +# then free to emit into kernels that run on any AVX512F host -- a Skylake-SP +# among them, where `vpdpbusd` does not exist. VNNI is its own level instead, so +# that the budget and the promise line up. set(SVS_ISA_LEVELS "AVX2|haswell|avx2" - "AVX512|cascadelake|avx512" + "AVX512|skylake-avx512|avx512" + "AVX512_VNNI|cascadelake|vnni" ) diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake index b1aa9a7c6..75a06c691 100644 --- a/cmake/generate-dispatch-surface.cmake +++ b/cmake/generate-dispatch-surface.cmake @@ -68,6 +68,7 @@ string(APPEND SVS_GEN_DIM_LOOP " /* end */") set(SVS_GEN_TARGET_LOOP "\\\n") set(SVS_GEN_LEVEL_LOOP "\\\n") +set(SVS_GEN_LEVEL_DEFINES "") set(SVS_DISPATCH_TU_SPECS) foreach(level_spec IN LISTS SVS_ISA_LEVELS) string(REPLACE "|" ";" level_fields "${level_spec}") @@ -76,6 +77,8 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) list(GET level_fields 2 infix) string(APPEND SVS_GEN_LEVEL_LOOP " M(${level}) \\\n") + string(APPEND SVS_GEN_LEVEL_DEFINES "#define SVS_ISA_LEVEL_${level} 1\n") + foreach(dim IN LISTS SVS_DIM_LIST) string(APPEND SVS_GEN_TARGET_LOOP " M(${dim}, ${level}) \\\n") endforeach() @@ -91,6 +94,7 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) endforeach() string(APPEND SVS_GEN_TARGET_LOOP " /* end */") string(APPEND SVS_GEN_LEVEL_LOOP " /* end */") +string(STRIP "${SVS_GEN_LEVEL_DEFINES}" SVS_GEN_LEVEL_DEFINES) ##### ##### Emit the header diff --git a/cmake/multi-arch.cmake b/cmake/multi-arch.cmake index 74be6c57f..e10e6e6c5 100644 --- a/cmake/multi-arch.cmake +++ b/cmake/multi-arch.cmake @@ -30,7 +30,9 @@ foreach(tu_spec IN LISTS SVS_DISPATCH_TU_SPECS) add_library(${lib_name} INTERFACE) target_compile_options(${lib_name} INTERFACE -march=${arch} -mtune=${arch}) - set(obj_name ${arch}_obj) + # Named after the level, not the -march: more than one level can share an + # instruction budget, and it is the level that says what is inside. + set(obj_name ${infix}_obj) add_library(${obj_name} OBJECT ${src}) target_link_libraries( ${obj_name} PRIVATE ${SVS_LIB} svs::compile_options fmt::fmt ${lib_name} diff --git a/cmake/templates/dispatch_surface.h.in b/cmake/templates/dispatch_surface.h.in index 64acd976d..ffbb9c50a 100644 --- a/cmake/templates/dispatch_surface.h.in +++ b/cmake/templates/dispatch_surface.h.in @@ -37,4 +37,8 @@ // enumerators without a translation unit are absent: this is the surface. #define SVS_FOR_EACH_ISA_LEVEL(M) @SVS_GEN_LEVEL_LOOP@ +// One per ISA level in the surface, and only those: a level absent from it has no +// instantiations. Tested with `defined` so a -Wundef build stays quiet. +@SVS_GEN_LEVEL_DEFINES@ + // clang-format on diff --git a/include/svs/core/distance/cosine.h b/include/svs/core/distance/cosine.h index 4cd8e3780..181b2d6c1 100644 --- a/include/svs/core/distance/cosine.h +++ b/include/svs/core/distance/cosine.h @@ -47,6 +47,20 @@ class CosineSimilarity { public: template static constexpr float compute(const Ea* a, const Eb* b, float a_norm, size_t N) { + // Compiles away entirely for the pairs with no VNNI kernel, which is most + // of them. + if constexpr (has_vnni_kernel) { + if (__builtin_expect( + svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 + )) { + return CosineSimilarityImpl< + Dynamic, + Ea, + Eb, + AVX_AVAILABILITY::AVX512_VNNI>:: + compute(a, b, a_norm, lib::MaybeStatic(N)); + } + } if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512f_supported(), 1)) { return CosineSimilarityImpl::compute( a, b, a_norm, lib::MaybeStatic(N) @@ -65,6 +79,23 @@ class CosineSimilarity { template static constexpr float compute(const Ea* a, const Eb* b, float a_norm) { + if constexpr (has_vnni_kernel) { + if (__builtin_expect( + svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 + )) { + if constexpr (is_dim_supported()) { + return CosineSimilarityImpl:: + compute(a, b, a_norm, lib::MaybeStatic()); + } else { + return CosineSimilarityImpl< + Dynamic, + Ea, + Eb, + AVX_AVAILABILITY::AVX512_VNNI>:: + compute(a, b, a_norm, lib::MaybeStatic(N)); + } + } + } if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512f_supported(), 1)) { if constexpr (is_dim_supported()) { return CosineSimilarityImpl::compute( @@ -255,38 +286,71 @@ template <> struct CosineFloatOp<16> : public svs::simd::ConvertToFloat<16> { } }; -// Small Integers +// Small Integers, with VNNI. Its own ISA level rather than a runtime branch inside +// the AVX512 kernels: the check belongs at the one place the level is chosen. SVS_VALIDATE_BOOL_ENV(SVS_AVX512_VNNI) #if SVS_AVX512_VNNI template -struct CosineSimilarityImpl { +struct CosineSimilarityImpl { SVS_NOINLINE static float compute(const int8_t* a, const int8_t* b, float a_norm, lib::MaybeStatic length) { - if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1)) { - auto sum = _mm512_setzero_epi32(); - auto bnorm_accum = _mm512_setzero_epi32(); - auto mask = create_mask<32>(length); - auto all = no_mask<32>(); - - for (size_t j = 0; j < length.size(); j += 32) { - auto temp_a = - _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, a + j); - auto va = _mm512_cvtepi8_epi16(temp_a); - - auto temp_b = - _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, b + j); - auto vb = _mm512_cvtepi8_epi16(temp_b); - - bnorm_accum = _mm512_dpwssd_epi32(bnorm_accum, vb, vb); - sum = _mm512_dpwssd_epi32(sum, va, vb); - } + auto sum = _mm512_setzero_epi32(); + auto bnorm_accum = _mm512_setzero_epi32(); + auto mask = create_mask<32>(length); + auto all = no_mask<32>(); + + for (size_t j = 0; j < length.size(); j += 32) { + auto temp_a = + _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, a + j); + auto va = _mm512_cvtepi8_epi16(temp_a); + + auto temp_b = + _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, b + j); + auto vb = _mm512_cvtepi8_epi16(temp_b); + + bnorm_accum = _mm512_dpwssd_epi32(bnorm_accum, vb, vb); + sum = _mm512_dpwssd_epi32(sum, va, vb); + } + + float b_norm = std::sqrt(static_cast(_mm512_reduce_add_epi32(bnorm_accum))); + return lib::narrow_cast(_mm512_reduce_add_epi32(sum)) / (a_norm * b_norm); + } +}; - float b_norm = - std::sqrt(static_cast(_mm512_reduce_add_epi32(bnorm_accum))); - return lib::narrow_cast(_mm512_reduce_add_epi32(sum)) / - (a_norm * b_norm); +template +struct CosineSimilarityImpl { + SVS_NOINLINE static float + compute(const uint8_t* a, const uint8_t* b, float a_norm, lib::MaybeStatic length) { + auto sum = _mm512_setzero_epi32(); + auto bnorm_accum = _mm512_setzero_epi32(); + auto mask = create_mask<32>(length); + auto all = no_mask<32>(); + + for (size_t j = 0; j < length.size(); j += 32) { + auto temp_a = + _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, a + j); + auto va = _mm512_cvtepu8_epi16(temp_a); + + auto temp_b = + _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, b + j); + auto vb = _mm512_cvtepu8_epi16(temp_b); + + bnorm_accum = _mm512_dpwssd_epi32(bnorm_accum, vb, vb); + sum = _mm512_dpwssd_epi32(sum, va, vb); } - // Fallback to AVX512 + float b_norm = std::sqrt(static_cast(_mm512_reduce_add_epi32(bnorm_accum))); + return lib::narrow_cast(_mm512_reduce_add_epi32(sum)) / (a_norm * b_norm); + } +}; + +#endif + +// Outside the SVS_AVX512_VNNI guard deliberately: the AVX512 translation unit is +// compiled where that macro is 0, so a missing specialization silently goes generic. +template +struct CosineSimilarityImpl { + SVS_NOINLINE static float + compute(const int8_t* a, const int8_t* b, float a_norm, lib::MaybeStatic length) { auto [sum, norm] = simd::generic_simd_op(CosineFloatOp<16>(), a, b, length); return sum / (std::sqrt(norm) * a_norm); } @@ -296,37 +360,11 @@ template struct CosineSimilarityImpl { SVS_NOINLINE static float compute(const uint8_t* a, const uint8_t* b, float a_norm, lib::MaybeStatic length) { - if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1)) { - auto sum = _mm512_setzero_epi32(); - auto bnorm_accum = _mm512_setzero_epi32(); - auto mask = create_mask<32>(length); - auto all = no_mask<32>(); - - for (size_t j = 0; j < length.size(); j += 32) { - auto temp_a = - _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, a + j); - auto va = _mm512_cvtepu8_epi16(temp_a); - - auto temp_b = - _mm256_maskz_loadu_epi8(islast<32>(length, j) ? mask : all, b + j); - auto vb = _mm512_cvtepu8_epi16(temp_b); - - bnorm_accum = _mm512_dpwssd_epi32(bnorm_accum, vb, vb); - sum = _mm512_dpwssd_epi32(sum, va, vb); - } - float b_norm = - std::sqrt(static_cast(_mm512_reduce_add_epi32(bnorm_accum))); - return lib::narrow_cast(_mm512_reduce_add_epi32(sum)) / - (a_norm * b_norm); - } - // Fallback to AVX512 auto [sum, norm] = simd::generic_simd_op(CosineFloatOp<16>(), a, b, length); return sum / (std::sqrt(norm) * a_norm); } }; -#endif - // Floating and Mixed Types template struct CosineSimilarityImpl { SVS_NOINLINE static float diff --git a/include/svs/core/distance/dispatch_surface.h b/include/svs/core/distance/dispatch_surface.h index a357ac0a8..152e6d921 100644 --- a/include/svs/core/distance/dispatch_surface.h +++ b/include/svs/core/distance/dispatch_surface.h @@ -60,6 +60,15 @@ M(512, AVX512) \ M(768, AVX512) \ M(svs::Dynamic, AVX512) \ + M(64, AVX512_VNNI) \ + M(96, AVX512_VNNI) \ + M(100, AVX512_VNNI) \ + M(128, AVX512_VNNI) \ + M(160, AVX512_VNNI) \ + M(200, AVX512_VNNI) \ + M(512, AVX512_VNNI) \ + M(768, AVX512_VNNI) \ + M(svs::Dynamic, AVX512_VNNI) \ /* end */ // Invokes M(isa_level) once per ISA level, weakest first. AVX_AVAILABILITY @@ -67,6 +76,13 @@ #define SVS_FOR_EACH_ISA_LEVEL(M) \ M(AVX2) \ M(AVX512) \ + M(AVX512_VNNI) \ /* end */ +// One per ISA level in the surface, and only those: a level absent from it has no +// instantiations. Tested with `defined` so a -Wundef build stays quiet. +#define SVS_ISA_LEVEL_AVX2 1 +#define SVS_ISA_LEVEL_AVX512 1 +#define SVS_ISA_LEVEL_AVX512_VNNI 1 + // clang-format on diff --git a/include/svs/core/distance/distance_core.h b/include/svs/core/distance/distance_core.h index 5d50dee1f..80ba089be 100644 --- a/include/svs/core/distance/distance_core.h +++ b/include/svs/core/distance/distance_core.h @@ -24,12 +24,41 @@ // The extent list and the ISA levels, generated from cmake/dispatch-surface.cmake. #include "svs/core/distance/dispatch_surface.h" +// Needed here and not only where the kernels are declared: the entry points must +// not dispatch to a level with no kernel for the pair in hand. +#if defined(__x86_64__) +#include "svs/multi-arch/x86/preprocessor.h" + +// Dispatched to for every type pair, so a surface without them would leave each +// consumer to instantiate the kernels itself, from the generic template. +#if !defined(SVS_ISA_LEVEL_AVX2) || !defined(SVS_ISA_LEVEL_AVX512) +#error "the x86 dispatch surface must declare the AVX2 and AVX512 ISA levels" +#endif +#endif + #include #include namespace svs::distance { -enum class AVX_AVAILABILITY { NONE, AVX2, AVX512 }; +/// The runtime ISA levels the library compiles distance kernels for. +/// +/// Each is a promise about the host, checked once in the entry point; the kernels +/// branch on nothing. Append new levels -- renumbering changes mangled names. +enum class AVX_AVAILABILITY { NONE, AVX2, AVX512, AVX512_VNNI }; + +/// Whether (Ea, Eb) has a kernel at AVX_AVAILABILITY::AVX512_VNNI. +/// +/// False where there is no such kernel -- a float-promoting pair, or any pair when +/// the surface omits the level. Dispatching anyway instantiates the generic template. +template inline constexpr bool has_vnni_kernel = false; + +#if defined(__x86_64__) && defined(SVS_ISA_LEVEL_AVX512_VNNI) +#define SVS_MARK_VNNI_PAIR(Ea, Eb, ...) \ + template <> inline constexpr bool has_vnni_kernel = true; +SVS_TYPE_PAIRS_AVX512_VNNI(SVS_MARK_VNNI_PAIR, ) +#undef SVS_MARK_VNNI_PAIR +#endif /// The extents that have a fixed-extent kernel, including svs::Dynamic. #define SVS_DIM_LIST_ENTRY(N) N, diff --git a/include/svs/core/distance/euclidean.h b/include/svs/core/distance/euclidean.h index a2fa68483..4363739c3 100644 --- a/include/svs/core/distance/euclidean.h +++ b/include/svs/core/distance/euclidean.h @@ -86,6 +86,17 @@ class L2 { public: template static constexpr float compute(const Ea* a, const Eb* b, size_t N) { + // Compiles away entirely for the pairs with no VNNI kernel, which is most + // of them. + if constexpr (has_vnni_kernel) { + if (__builtin_expect( + svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 + )) { + return L2Impl::compute( + a, b, lib::MaybeStatic(N) + ); + } + } if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512f_supported(), 1)) { return L2Impl::compute( a, b, lib::MaybeStatic(N) @@ -103,6 +114,21 @@ class L2 { template static constexpr float compute(const Ea* a, const Eb* b) { + if constexpr (has_vnni_kernel) { + if (__builtin_expect( + svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 + )) { + if constexpr (is_dim_supported()) { + return L2Impl::compute( + a, b, lib::MaybeStatic() + ); + } else { + return L2Impl::compute( + a, b, lib::MaybeStatic(N) + ); + } + } + } if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512f_supported(), 1)) { if constexpr (is_dim_supported()) { return L2Impl::compute( @@ -258,7 +284,8 @@ template <> struct L2FloatOp<16> : public svs::simd::ConvertToFloat<16> { static float reduce(__m512 x) { return _mm512_reduce_add_ps(x); } }; -// Small Integers +// Small Integers, with VNNI. Its own ISA level rather than a runtime branch inside +// the AVX512 kernels: the check belongs at the one place the level is chosen. SVS_VALIDATE_BOOL_ENV(SVS_AVX512_VNNI) #if SVS_AVX512_VNNI @@ -289,14 +316,27 @@ template <> struct L2VNNIOp : public svs::simd::ConvertForVNNI struct L2Impl { + SVS_NOINLINE static float + compute(const int8_t* a, const int8_t* b, lib::MaybeStatic length) { + return simd::generic_simd_op(L2VNNIOp(), a, b, length); + } +}; + +template struct L2Impl { + SVS_NOINLINE static float + compute(const uint8_t* a, const uint8_t* b, lib::MaybeStatic length) { + return simd::generic_simd_op(L2VNNIOp(), a, b, length); + } +}; + +#endif + +// Outside the SVS_AVX512_VNNI guard deliberately: the AVX512 translation unit is +// compiled where that macro is 0, so a missing specialization silently goes generic. template struct L2Impl { SVS_NOINLINE static float compute(const int8_t* a, const int8_t* b, lib::MaybeStatic length) { - if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1)) { - return simd::generic_simd_op(L2VNNIOp(), a, b, length); - } - // fallback to AVX512 return simd::generic_simd_op(L2FloatOp<16>{}, a, b, length); } }; @@ -304,16 +344,10 @@ template struct L2Impl { template struct L2Impl { SVS_NOINLINE static float compute(const uint8_t* a, const uint8_t* b, lib::MaybeStatic length) { - if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1)) { - return simd::generic_simd_op(L2VNNIOp(), a, b, length); - } - // fallback to AVX512 return simd::generic_simd_op(L2FloatOp<16>{}, a, b, length); } }; -#endif - // Floating and Mixed Types template struct L2Impl { SVS_NOINLINE static float diff --git a/include/svs/core/distance/inner_product.h b/include/svs/core/distance/inner_product.h index 14293cb38..a6d17d119 100644 --- a/include/svs/core/distance/inner_product.h +++ b/include/svs/core/distance/inner_product.h @@ -46,6 +46,17 @@ class IP { public: template static constexpr float compute(const Ea* a, const Eb* b, size_t N) { + // Compiles away entirely for the pairs with no VNNI kernel, which is most + // of them. + if constexpr (has_vnni_kernel) { + if (__builtin_expect( + svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 + )) { + return IPImpl::compute( + a, b, lib::MaybeStatic(N) + ); + } + } if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512f_supported(), 1)) { return IPImpl::compute( a, b, lib::MaybeStatic(N) @@ -63,6 +74,21 @@ class IP { template static constexpr float compute(const Ea* a, const Eb* b) { + if constexpr (has_vnni_kernel) { + if (__builtin_expect( + svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 + )) { + if constexpr (is_dim_supported()) { + return IPImpl::compute( + a, b, lib::MaybeStatic() + ); + } else { + return IPImpl::compute( + a, b, lib::MaybeStatic(N) + ); + } + } + } if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512f_supported(), 1)) { if constexpr (is_dim_supported()) { return IPImpl::compute( @@ -215,7 +241,8 @@ template <> struct IPFloatOp<16> : public svs::simd::ConvertToFloat<16> { static float reduce(__m512 x) { return _mm512_reduce_add_ps(x); } }; -// Small Integers +// Small Integers, with VNNI. Its own ISA level rather than a runtime branch inside +// the AVX512 kernels: the check belongs at the one place the level is chosen. SVS_VALIDATE_BOOL_ENV(SVS_AVX512_VNNI) #if SVS_AVX512_VNNI @@ -243,14 +270,27 @@ template <> struct IPVNNIOp : public svs::simd::ConvertForVNNI struct IPImpl { + SVS_NOINLINE static float + compute(const int8_t* a, const int8_t* b, lib::MaybeStatic length) { + return simd::generic_simd_op(IPVNNIOp(), a, b, length); + } +}; + +template struct IPImpl { + SVS_NOINLINE static float + compute(const uint8_t* a, const uint8_t* b, lib::MaybeStatic length) { + return simd::generic_simd_op(IPVNNIOp(), a, b, length); + } +}; + +#endif + +// Outside the SVS_AVX512_VNNI guard deliberately: the AVX512 translation unit is +// compiled where that macro is 0, so a missing specialization silently goes generic. template struct IPImpl { SVS_NOINLINE static float compute(const int8_t* a, const int8_t* b, lib::MaybeStatic length) { - if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1)) { - return simd::generic_simd_op(IPVNNIOp(), a, b, length); - } - // fallback to AVX512 return svs::simd::generic_simd_op(IPFloatOp<16>{}, a, b, length); } }; @@ -258,16 +298,10 @@ template struct IPImpl { template struct IPImpl { SVS_NOINLINE static float compute(const uint8_t* a, const uint8_t* b, lib::MaybeStatic length) { - if (__builtin_expect(svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1)) { - return simd::generic_simd_op(IPVNNIOp(), a, b, length); - } - // fallback to AVX512 return svs::simd::generic_simd_op(IPFloatOp<16>{}, a, b, length); } }; -#endif - // Floating and Mixed Types template struct IPImpl { SVS_NOINLINE static float diff --git a/include/svs/multi-arch/x86/preprocessor.h b/include/svs/multi-arch/x86/preprocessor.h index 75941b70f..82eccd846 100644 --- a/include/svs/multi-arch/x86/preprocessor.h +++ b/include/svs/multi-arch/x86/preprocessor.h @@ -55,6 +55,12 @@ #define SVS_TYPE_PAIRS_AVX2 SVS_FOR_EACH_TYPE_PAIR #define SVS_TYPE_PAIRS_AVX512 SVS_FOR_EACH_TYPE_PAIR +// Only these two: every other pair promotes to float, where VNNI has nothing to +// offer. svs::distance::has_vnni_kernel is generated from this same list. +#define SVS_TYPE_PAIRS_AVX512_VNNI(M, ...) \ + M(int8_t, int8_t, __VA_ARGS__) \ + M(uint8_t, uint8_t, __VA_ARGS__) + ///// ///// Instantiation. ///// diff --git a/include/svs/multi-arch/x86/vnni.cpp b/include/svs/multi-arch/x86/vnni.cpp new file mode 100644 index 000000000..1b719f8ac --- /dev/null +++ b/include/svs/multi-arch/x86/vnni.cpp @@ -0,0 +1,32 @@ +/* + * Copyright 2026 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. + */ + +#if defined(__x86_64__) +#include "svs/core/distance/cosine.h" +#include "svs/core/distance/euclidean.h" +#include "svs/core/distance/inner_product.h" + +namespace svs::distance { + +// Define every kernel for this ISA level at every generated extent. Only the two +// small integer pairs, which is why a fourth level costs 54 kernels, not 432. +#define SVS_DEFINE_FOR_DIM(DIM) SVS_INSTANTIATE_DISTANCES(template, DIM, AVX512_VNNI) +SVS_FOR_EACH_SUPPORTED_DIM(SVS_DEFINE_FOR_DIM) +#undef SVS_DEFINE_FOR_DIM + +} // namespace svs::distance + +#endif diff --git a/tests/cmake/dispatch-surface/valid-reduced.cmake b/tests/cmake/dispatch-surface/valid-reduced.cmake index 4a9bfcc04..dcabc8e54 100644 --- a/tests/cmake/dispatch-surface/valid-reduced.cmake +++ b/tests/cmake/dispatch-surface/valid-reduced.cmake @@ -13,13 +13,18 @@ # 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. +# it cannot accidentally pass by reusing a committed header. +# +# The extent list is what this fixture varies. The ISA levels are copied from the +# default declaration verbatim, instruction budgets included: they are not +# configuration, and a build with a different level set would be testing a library +# nobody ships. # # 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" + "AVX512|skylake-avx512|avx512" + "AVX512_VNNI|cascadelake|vnni" ) diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index d52c1bc2e..744dfc404 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -112,7 +112,7 @@ if(svs_objdump) NAME dispatch_instructions_${svs_infix} COMMAND "${CMAKE_COMMAND}" - "-DSVS_OBJECT=$" + "-DSVS_OBJECT=$" "-DSVS_LEVEL=${svs_level}" "-DSVS_ARCH=${svs_arch}" "-DSVS_OBJDUMP=${svs_objdump}" diff --git a/tests/multi-arch/x86/entry_probe.cpp b/tests/multi-arch/x86/entry_probe.cpp index 567ae6784..f55719106 100644 --- a/tests/multi-arch/x86/entry_probe.cpp +++ b/tests/multi-arch/x86/entry_probe.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -98,9 +99,11 @@ int main(int argc, char** argv) { // breaks on this extent's kernels to see which level the call enters. std::printf("expect-level %d\n", svs_test::expected_level()); std::printf("probe-extent %zu\n", entry_report_dim); + // int8/int8, mangled `aa` and matched in cmake/check-dispatch-execution.cmake: + // a float-promoting pair has no AVX512_VNNI kernel and routes a level lower. std::printf( "one-call %f\n", - static_cast(entry_one()) + static_cast(entry_one()) ); return 0; } diff --git a/tests/multi-arch/x86/host_levels.h b/tests/multi-arch/x86/host_levels.h index a94c42973..b8372655e 100644 --- a/tests/multi-arch/x86/host_levels.h +++ b/tests/multi-arch/x86/host_levels.h @@ -38,6 +38,10 @@ template <> inline bool host_satisfies() { return svs::detail::avx_runtime_flags.is_avx512f_supported(); } +template <> inline bool host_satisfies() { + return svs::detail::avx_runtime_flags.is_avx512vnni_supported(); +} + /// The level the distance entry points must choose on this host. /// /// The entry points test the strongest level first, so their choice is the diff --git a/tests/multi-arch/x86/link_probe.cpp b/tests/multi-arch/x86/link_probe.cpp index 443c87e62..ea540a92d 100644 --- a/tests/multi-arch/x86/link_probe.cpp +++ b/tests/multi-arch/x86/link_probe.cpp @@ -86,9 +86,11 @@ template svs::lib::MaybeStatic probe_length() { 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) \ +// The level's own type-pair list, not the full one: naming a pair the level has no +// kernel for would instantiate the generic template right here. +#define SVS_PROBE_TARGET(N, LEVEL) \ + if (host_satisfies()) { \ + SVS_TYPE_PAIRS_FOR(LEVEL, SVS_PROBE_ONE, N, LEVEL) \ } float probe_all() { From 2cdeb0a16e21d53a5690c97ec2ff932de79d7969 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Tue, 25 Aug 2026 08:01:14 -0700 Subject: [PATCH 09/23] improve comments --- include/svs/core/distance/cosine.h | 9 +++------ include/svs/core/distance/euclidean.h | 9 +++------ include/svs/core/distance/inner_product.h | 9 +++------ include/svs/multi-arch/x86/vnni.cpp | 4 ++-- 4 files changed, 11 insertions(+), 20 deletions(-) diff --git a/include/svs/core/distance/cosine.h b/include/svs/core/distance/cosine.h index 181b2d6c1..75f617ab1 100644 --- a/include/svs/core/distance/cosine.h +++ b/include/svs/core/distance/cosine.h @@ -47,8 +47,6 @@ class CosineSimilarity { public: template static constexpr float compute(const Ea* a, const Eb* b, float a_norm, size_t N) { - // Compiles away entirely for the pairs with no VNNI kernel, which is most - // of them. if constexpr (has_vnni_kernel) { if (__builtin_expect( svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 @@ -286,8 +284,7 @@ template <> struct CosineFloatOp<16> : public svs::simd::ConvertToFloat<16> { } }; -// Small Integers, with VNNI. Its own ISA level rather than a runtime branch inside -// the AVX512 kernels: the check belongs at the one place the level is chosen. +// Small Integers, with VNNI SVS_VALIDATE_BOOL_ENV(SVS_AVX512_VNNI) #if SVS_AVX512_VNNI template @@ -345,8 +342,8 @@ struct CosineSimilarityImpl #endif -// Outside the SVS_AVX512_VNNI guard deliberately: the AVX512 translation unit is -// compiled where that macro is 0, so a missing specialization silently goes generic. +// Must stay outside the SVS_AVX512_VNNI guard: avx512.cpp compiles with that macro +// at 0, and hiding this specialization there would silently select the generic kernel. template struct CosineSimilarityImpl { SVS_NOINLINE static float diff --git a/include/svs/core/distance/euclidean.h b/include/svs/core/distance/euclidean.h index 4363739c3..1d3734192 100644 --- a/include/svs/core/distance/euclidean.h +++ b/include/svs/core/distance/euclidean.h @@ -86,8 +86,6 @@ class L2 { public: template static constexpr float compute(const Ea* a, const Eb* b, size_t N) { - // Compiles away entirely for the pairs with no VNNI kernel, which is most - // of them. if constexpr (has_vnni_kernel) { if (__builtin_expect( svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 @@ -284,8 +282,7 @@ template <> struct L2FloatOp<16> : public svs::simd::ConvertToFloat<16> { static float reduce(__m512 x) { return _mm512_reduce_add_ps(x); } }; -// Small Integers, with VNNI. Its own ISA level rather than a runtime branch inside -// the AVX512 kernels: the check belongs at the one place the level is chosen. +// Small Integers, with VNNI SVS_VALIDATE_BOOL_ENV(SVS_AVX512_VNNI) #if SVS_AVX512_VNNI @@ -332,8 +329,8 @@ template struct L2Impl struct L2Impl { SVS_NOINLINE static float compute(const int8_t* a, const int8_t* b, lib::MaybeStatic length) { diff --git a/include/svs/core/distance/inner_product.h b/include/svs/core/distance/inner_product.h index a6d17d119..c7d958a0d 100644 --- a/include/svs/core/distance/inner_product.h +++ b/include/svs/core/distance/inner_product.h @@ -46,8 +46,6 @@ class IP { public: template static constexpr float compute(const Ea* a, const Eb* b, size_t N) { - // Compiles away entirely for the pairs with no VNNI kernel, which is most - // of them. if constexpr (has_vnni_kernel) { if (__builtin_expect( svs::detail::avx_runtime_flags.is_avx512vnni_supported(), 1 @@ -241,8 +239,7 @@ template <> struct IPFloatOp<16> : public svs::simd::ConvertToFloat<16> { static float reduce(__m512 x) { return _mm512_reduce_add_ps(x); } }; -// Small Integers, with VNNI. Its own ISA level rather than a runtime branch inside -// the AVX512 kernels: the check belongs at the one place the level is chosen. +// Small Integers, with VNNI SVS_VALIDATE_BOOL_ENV(SVS_AVX512_VNNI) #if SVS_AVX512_VNNI @@ -286,8 +283,8 @@ template struct IPImpl struct IPImpl { SVS_NOINLINE static float compute(const int8_t* a, const int8_t* b, lib::MaybeStatic length) { diff --git a/include/svs/multi-arch/x86/vnni.cpp b/include/svs/multi-arch/x86/vnni.cpp index 1b719f8ac..9c2810b00 100644 --- a/include/svs/multi-arch/x86/vnni.cpp +++ b/include/svs/multi-arch/x86/vnni.cpp @@ -21,8 +21,8 @@ namespace svs::distance { -// Define every kernel for this ISA level at every generated extent. Only the two -// small integer pairs, which is why a fourth level costs 54 kernels, not 432. +// Define every kernel for this ISA level at every generated extent. Extents come +// from cmake/dispatch-surface.cmake, type pairs from multi-arch/x86/preprocessor.h. #define SVS_DEFINE_FOR_DIM(DIM) SVS_INSTANTIATE_DISTANCES(template, DIM, AVX512_VNNI) SVS_FOR_EACH_SUPPORTED_DIM(SVS_DEFINE_FOR_DIM) #undef SVS_DEFINE_FOR_DIM From c2421c44aa64befcea1acb14f97bcd01b7202708 Mon Sep 17 00:00:00 2001 From: Andreas Huber <9201869+ahuber21@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:07:34 +0200 Subject: [PATCH 10/23] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cmake/AGENTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/AGENTS.md b/cmake/AGENTS.md index 8b6da30b9..cd81f453e 100644 --- a/cmake/AGENTS.md +++ b/cmake/AGENTS.md @@ -9,8 +9,8 @@ Build modules, dependency wiring, and feature toggles. ## Intel-specific modules - **`cmake/mkl.cmake`:** MKL linkage (static vs dynamic threading). Do not hardcode MKL versions. When changing linkage mode, validate threading behavior in tests. -- **`cmake/multi-arch.cmake`:** AVX-512 / SIMD ISA dispatch. Do not hardcode `-march` or ISA flags outside this file. Changes must align with `include/svs/multi-arch/` runtime dispatch code. -- **`cmake/dispatch-surface.cmake`:** The declared x86 dispatch surface — the fixed extents and the ISA levels. Edit it, never the generated `include/svs/core/distance/dispatch_surface.h`, which every configure overwrites. `cmake/dispatch-checks/` holds the ctest checkers that hold the built binary to this declaration. +- **`cmake/multi-arch.cmake`:** AVX-512 / SIMD ISA dispatch wiring. Changes must align with `include/svs/multi-arch/` runtime dispatch code. +- **`cmake/dispatch-surface.cmake`:** The declared x86 dispatch surface — fixed extents, ISA levels, and their `-march` budgets. Edit it, never the generated `include/svs/core/distance/dispatch_surface.h`, which every configure overwrites. `cmake/dispatch-checks/` holds the ctest checkers that hold the built binary to this declaration. - **`cmake/numa.cmake`:** NUMA-aware memory allocation. Respect NUMA topology assumptions in performance-critical code. - **`cmake/openmp.cmake`:** Threading model. Do not assume specific OpenMP version or runtime without checking source-of-truth. From addd93f3b38053e813962a666af21bc77042d915 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 04:33:45 -0700 Subject: [PATCH 11/23] refactor(dispatch): move the surface checkers under tests and share their helpers The checkers are test drivers, so they belong under tests/multi-arch/ with the probes they run. Extract three divergent copies of symbol-reading logic into a shared lib.cmake, eliminating duplication of svs_nm_symbols, svs_require, svs_require_files, and svs_report_first. Co-Authored-By: Claude Opus 5 --- cmake/AGENTS.md | 2 +- tests/multi-arch/CMakeLists.txt | 2 +- .../multi-arch}/dispatch-checks/README.md | 0 .../check-dispatch-declaration.cmake | 69 +++--------------- .../check-dispatch-execution.cmake | 33 +++------ .../check-dispatch-instructions.cmake | 14 ++-- .../check-dispatch-linkage.cmake | 72 +++---------------- tests/multi-arch/dispatch-checks/lib.cmake | 71 ++++++++++++++++++ tests/multi-arch/x86/entry_probe.cpp | 2 +- 9 files changed, 108 insertions(+), 157 deletions(-) rename {cmake => tests/multi-arch}/dispatch-checks/README.md (100%) rename {cmake => tests/multi-arch}/dispatch-checks/check-dispatch-declaration.cmake (84%) rename {cmake => tests/multi-arch}/dispatch-checks/check-dispatch-execution.cmake (84%) rename {cmake => tests/multi-arch}/dispatch-checks/check-dispatch-instructions.cmake (93%) rename {cmake => tests/multi-arch}/dispatch-checks/check-dispatch-linkage.cmake (65%) create mode 100644 tests/multi-arch/dispatch-checks/lib.cmake diff --git a/cmake/AGENTS.md b/cmake/AGENTS.md index cd81f453e..227c2aa4c 100644 --- a/cmake/AGENTS.md +++ b/cmake/AGENTS.md @@ -10,7 +10,7 @@ Build modules, dependency wiring, and feature toggles. ## Intel-specific modules - **`cmake/mkl.cmake`:** MKL linkage (static vs dynamic threading). Do not hardcode MKL versions. When changing linkage mode, validate threading behavior in tests. - **`cmake/multi-arch.cmake`:** AVX-512 / SIMD ISA dispatch wiring. Changes must align with `include/svs/multi-arch/` runtime dispatch code. -- **`cmake/dispatch-surface.cmake`:** The declared x86 dispatch surface — fixed extents, ISA levels, and their `-march` budgets. Edit it, never the generated `include/svs/core/distance/dispatch_surface.h`, which every configure overwrites. `cmake/dispatch-checks/` holds the ctest checkers that hold the built binary to this declaration. +- **`cmake/dispatch-surface.cmake`:** The declared x86 dispatch surface — fixed extents, ISA levels, and their `-march` budgets. Edit it, never the generated `include/svs/core/distance/dispatch_surface.h`, which every configure overwrites. `tests/multi-arch/dispatch-checks/` holds the ctest checkers that hold the built binary to this declaration. - **`cmake/numa.cmake`:** NUMA-aware memory allocation. Respect NUMA topology assumptions in performance-critical code. - **`cmake/openmp.cmake`:** Threading model. Do not assume specific OpenMP version or runtime without checking source-of-truth. diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index 744dfc404..15e79c1bd 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -23,7 +23,7 @@ ##### directory to its own project, where that points somewhere else entirely. ##### -set(svs_dispatch_cmake_dir "${CMAKE_CURRENT_LIST_DIR}/../../cmake/dispatch-checks") +set(svs_dispatch_cmake_dir "${CMAKE_CURRENT_LIST_DIR}/dispatch-checks") # The object libraries exist so that each probe's single object file can be named # with $, which is what the symbol-table checks read. diff --git a/cmake/dispatch-checks/README.md b/tests/multi-arch/dispatch-checks/README.md similarity index 100% rename from cmake/dispatch-checks/README.md rename to tests/multi-arch/dispatch-checks/README.md diff --git a/cmake/dispatch-checks/check-dispatch-declaration.cmake b/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake similarity index 84% rename from cmake/dispatch-checks/check-dispatch-declaration.cmake rename to tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake index 4e50d83f7..929342945 100644 --- a/cmake/dispatch-checks/check-dispatch-declaration.cmake +++ b/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake @@ -24,9 +24,9 @@ ##### -DSVS_ARCHIVE= \ ##### -DSVS_CONSUMER_OBJECT= \ ##### -DSVS_NM= \ -##### -P cmake/dispatch-checks/check-dispatch-declaration.cmake +##### -P tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake ##### -##### cmake/dispatch-checks/check-dispatch-linkage.cmake compares the archive +##### tests/multi-arch/dispatch-checks/check-dispatch-linkage.cmake compares the archive ##### against a probe, and both are generated from the same header: a generator ##### that dropped an extent would drop it from both and still agree. The ##### expectation here is derived from the three hand-written sources instead -- @@ -39,22 +39,10 @@ # rejected as unknown arguments. cmake_minimum_required(VERSION 3.21) -foreach(required - SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER - SVS_ARCHIVE SVS_CONSUMER_OBJECT SVS_NM -) - if(NOT ${required}) - message(FATAL_ERROR "${required} is not set.") - endif() -endforeach() -foreach(required - SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER - SVS_ARCHIVE SVS_CONSUMER_OBJECT -) - if(NOT EXISTS "${${required}}") - message(FATAL_ERROR "${required} does not exist: ${${required}}") - endif() -endforeach() +include("${CMAKE_CURRENT_LIST_DIR}/lib.cmake") + +svs_require(SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER SVS_ARCHIVE SVS_CONSUMER_OBJECT SVS_NM) +svs_require_files(SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER SVS_ARCHIVE SVS_CONSUMER_OBJECT) ##### ##### Ground truth 1: the extents and the ISA levels @@ -187,29 +175,7 @@ endforeach() # Mangled names throughout: demangled ones carry `[clone .isra.0]` suffixes that # differ between a local instantiation and an explicit one. set(svs_kernel_regex "^_ZN3svs8distance([0-9]+[A-Za-z0-9_]+)ILm([0-9]+)E.*AVX_AVAILABILITYE([0-9]+)E") - -function(svs_symbols out_var file nm_arg) - execute_process( - COMMAND "${SVS_NM}" ${nm_arg} "${file}" - OUTPUT_VARIABLE raw - ERROR_VARIABLE err - RESULT_VARIABLE status - ) - if(NOT status EQUAL 0) - message(FATAL_ERROR "${SVS_NM} failed on ${file}: ${err}") - endif() - set(symbols) - string(REPLACE "\n" ";" lines "${raw}") - foreach(line IN LISTS lines) - if(line MATCHES "_ZN3svs8distance.*AVX_AVAILABILITY") - # 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) - set(${out_var} "${symbols}" PARENT_SCOPE) -endfunction() +set(svs_distance_regex "_ZN3svs8distance.*AVX_AVAILABILITY") # Sets _KEYS, _FOREIGN and _HAVE_ in the caller. function(svs_tally prefix symbols) @@ -236,9 +202,9 @@ function(svs_tally prefix symbols) set(${prefix}_FOREIGN "${foreign}" PARENT_SCOPE) endfunction() -svs_symbols(archive_defines "${SVS_ARCHIVE}" --defined-only) -svs_symbols(consumer_defines "${SVS_CONSUMER_OBJECT}" --defined-only) -svs_symbols(consumer_references "${SVS_CONSUMER_OBJECT}" --undefined-only) +svs_nm_symbols(archive_defines "${SVS_ARCHIVE}" "${svs_distance_regex}" NM_ARGS --defined-only) +svs_nm_symbols(consumer_defines "${SVS_CONSUMER_OBJECT}" "${svs_distance_regex}" NM_ARGS --defined-only) +svs_nm_symbols(consumer_references "${SVS_CONSUMER_OBJECT}" "${svs_distance_regex}" NM_ARGS --undefined-only) svs_tally(ARCHIVE "${archive_defines}") svs_tally(CONSUMER "${consumer_references}") @@ -249,21 +215,6 @@ svs_tally(CONSUMER "${consumer_references}") set(errors 0) -function(svs_report_first list_var limit) - set(shown ${${list_var}}) - list(LENGTH shown count) - if(count GREATER limit) - list(SUBLIST shown 0 ${limit} shown) - endif() - foreach(entry IN LISTS shown) - message(" ${entry}") - endforeach() - if(count GREATER limit) - math(EXPR rest "${count} - ${limit}") - message(" ... and ${rest} more") - endif() -endfunction() - # Sets _ERRORS in the caller. `what` names the thing being compared, for # the failure message. function(svs_compare prefix what) diff --git a/cmake/dispatch-checks/check-dispatch-execution.cmake b/tests/multi-arch/dispatch-checks/check-dispatch-execution.cmake similarity index 84% rename from cmake/dispatch-checks/check-dispatch-execution.cmake rename to tests/multi-arch/dispatch-checks/check-dispatch-execution.cmake index f16ff3a20..6d86e7171 100644 --- a/cmake/dispatch-checks/check-dispatch-execution.cmake +++ b/tests/multi-arch/dispatch-checks/check-dispatch-execution.cmake @@ -18,7 +18,7 @@ ##### Run in script mode: ##### ##### cmake -DSVS_PROBE= -DSVS_NM= -DSVS_GDB= \ -##### -P cmake/dispatch-checks/check-dispatch-execution.cmake +##### -P tests/multi-arch/dispatch-checks/check-dispatch-execution.cmake ##### ##### Everything else about the surface is a property of the symbol table, which a ##### specialization can satisfy while never running: one that disappears behind an @@ -29,14 +29,10 @@ ##### hosts that satisfy only those, which is what the CI matrix is for. ##### -foreach(required SVS_PROBE SVS_NM SVS_GDB) - if(NOT ${required}) - message(FATAL_ERROR "${required} is not set.") - endif() -endforeach() -if(NOT EXISTS "${SVS_PROBE}") - message(FATAL_ERROR "SVS_PROBE does not exist: ${SVS_PROBE}") -endif() +include("${CMAKE_CURRENT_LIST_DIR}/lib.cmake") + +svs_require(SVS_PROBE SVS_NM SVS_GDB) +svs_require_files(SVS_PROBE) # The probe owns the runtime predicates, so it -- not this script -- decides which # level the entry points are obliged to choose here. @@ -76,24 +72,11 @@ set(svs_probe_pair "aa") # L2 at that pair for the extent the probe reports: one symbol per level, including # any AVX_AVAILABILITY::NONE fallback the consumer instantiated itself. -execute_process( - COMMAND "${SVS_NM}" --defined-only "${SVS_PROBE}" - OUTPUT_VARIABLE raw - ERROR_VARIABLE err - RESULT_VARIABLE status -) -if(NOT status EQUAL 0) - message(FATAL_ERROR "${SVS_NM} failed on ${SVS_PROBE}: ${err}") -endif() +set(svs_symbol_regex "_ZN3svs8distance6L2ImplILm${extent}E${svs_probe_pair}.*7computeE") +svs_nm_symbols(raw_symbols "${SVS_PROBE}" "${svs_symbol_regex}" NM_ARGS --defined-only) set(candidates) -string(REPLACE "\n" ";" lines "${raw}") -foreach(line IN LISTS lines) - # The mangled name is the last whitespace-separated field. - string(REGEX MATCH "[^ \t]+$" symbol "${line}") - if(NOT symbol MATCHES "^_ZN3svs8distance6L2ImplILm${extent}E${svs_probe_pair}.*7computeE") - continue() - endif() +foreach(symbol IN LISTS raw_symbols) # Skip GCC's `.isra` clones: gdb reads a dot in a linespec as a file name. if(symbol MATCHES "\\.") continue() diff --git a/cmake/dispatch-checks/check-dispatch-instructions.cmake b/tests/multi-arch/dispatch-checks/check-dispatch-instructions.cmake similarity index 93% rename from cmake/dispatch-checks/check-dispatch-instructions.cmake rename to tests/multi-arch/dispatch-checks/check-dispatch-instructions.cmake index 45e1f0698..cd2b93fa3 100644 --- a/cmake/dispatch-checks/check-dispatch-instructions.cmake +++ b/tests/multi-arch/dispatch-checks/check-dispatch-instructions.cmake @@ -20,7 +20,7 @@ ##### cmake -DSVS_OBJECT= \ ##### -DSVS_LEVEL=AVX2 -DSVS_ARCH=haswell \ ##### -DSVS_OBJDUMP= \ -##### -P cmake/dispatch-checks/check-dispatch-instructions.cmake +##### -P tests/multi-arch/dispatch-checks/check-dispatch-instructions.cmake ##### ##### A level promises the host satisfies its runtime predicate and nothing more, ##### so an instruction the predicate does not guarantee is an illegal-instruction @@ -31,14 +31,10 @@ # field of a budget row below is silently dropped rather than read as empty. cmake_minimum_required(VERSION 3.21) -foreach(required SVS_OBJECT SVS_LEVEL SVS_ARCH SVS_OBJDUMP) - if(NOT ${required}) - message(FATAL_ERROR "${required} is not set.") - endif() -endforeach() -if(NOT EXISTS "${SVS_OBJECT}") - message(FATAL_ERROR "SVS_OBJECT does not exist: ${SVS_OBJECT}") -endif() +include("${CMAKE_CURRENT_LIST_DIR}/lib.cmake") + +svs_require(SVS_OBJECT SVS_LEVEL SVS_ARCH SVS_OBJDUMP) +svs_require_files(SVS_OBJECT) # What each instruction class looks like in AT&T disassembly. Register classes are # matched with their `%` sigil so that a mangled name can never look like one. diff --git a/cmake/dispatch-checks/check-dispatch-linkage.cmake b/tests/multi-arch/dispatch-checks/check-dispatch-linkage.cmake similarity index 65% rename from cmake/dispatch-checks/check-dispatch-linkage.cmake rename to tests/multi-arch/dispatch-checks/check-dispatch-linkage.cmake index 6ec22b711..9fa9ae2cb 100644 --- a/cmake/dispatch-checks/check-dispatch-linkage.cmake +++ b/tests/multi-arch/dispatch-checks/check-dispatch-linkage.cmake @@ -20,7 +20,7 @@ ##### cmake -DSVS_PROBE_OBJECT= \ ##### -DSVS_ARCHIVE= \ ##### -DSVS_NM= \ -##### -P cmake/dispatch-checks/check-dispatch-linkage.cmake +##### -P tests/multi-arch/dispatch-checks/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* @@ -29,69 +29,19 @@ ##### 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() +include("${CMAKE_CURRENT_LIST_DIR}/lib.cmake") + +svs_require(SVS_PROBE_OBJECT SVS_ARCHIVE SVS_NM) +svs_require_files(SVS_PROBE_OBJECT SVS_ARCHIVE) # 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) +svs_nm_symbols(probe_defines "${SVS_PROBE_OBJECT}" "${svs_kernel_regex}" NM_ARGS --defined-only) +svs_nm_symbols(probe_references "${SVS_PROBE_OBJECT}" "${svs_kernel_regex}" NM_ARGS --undefined-only) +svs_nm_symbols(archive_defines "${SVS_ARCHIVE}" "${svs_kernel_regex}" NM_ARGS --defined-only) list(LENGTH probe_defines n_probe_defines) list(LENGTH probe_references n_probe_references) @@ -118,7 +68,7 @@ if(NOT n_probe_defines EQUAL 0) 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) + svs_report_first(probe_defines 10) math(EXPR errors "${errors} + 1") endif() @@ -134,7 +84,7 @@ if(NOT n_missing EQUAL 0) 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) + svs_report_first(missing 10) math(EXPR errors "${errors} + 1") endif() @@ -149,7 +99,7 @@ if(NOT n_unreachable EQUAL 0) 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) + svs_report_first(unreachable 10) math(EXPR errors "${errors} + 1") endif() diff --git a/tests/multi-arch/dispatch-checks/lib.cmake b/tests/multi-arch/dispatch-checks/lib.cmake new file mode 100644 index 000000000..9bcb35446 --- /dev/null +++ b/tests/multi-arch/dispatch-checks/lib.cmake @@ -0,0 +1,71 @@ +# Copyright 2026 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. + +include_guard(GLOBAL) + +function(svs_require) # svs_require(VAR_A VAR_B ...) + foreach(var IN LISTS ARGN) + if(NOT ${var}) + message(FATAL_ERROR "${var} is not set.") + endif() + endforeach() +endfunction() + +function(svs_require_files) # svs_require_files(VAR_A VAR_B ...) + foreach(var IN LISTS ARGN) + if(NOT EXISTS "${${var}}") + message(FATAL_ERROR "${var} does not exist: ${${var}}") + endif() + endforeach() +endfunction() + +function(svs_nm_symbols out_var file regex) + cmake_parse_arguments(arg "" "" "NM_ARGS" ${ARGN}) + execute_process( + COMMAND "${SVS_NM}" ${arg_NM_ARGS} "${file}" + OUTPUT_VARIABLE raw + ERROR_VARIABLE err + RESULT_VARIABLE status + ) + if(NOT status EQUAL 0) + message(FATAL_ERROR "${SVS_NM} failed on ${file}: ${err}") + endif() + + set(symbols) + string(REPLACE "\n" ";" lines "${raw}") + foreach(line IN LISTS lines) + if(line MATCHES "${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) + set(${out_var} "${symbols}" PARENT_SCOPE) +endfunction() + +function(svs_report_first list_var limit) + set(shown ${${list_var}}) + list(LENGTH shown count) + if(count GREATER limit) + list(SUBLIST shown 0 ${limit} shown) + endif() + foreach(entry IN LISTS shown) + message(" ${entry}") + endforeach() + if(count GREATER limit) + math(EXPR rest "${count} - ${limit}") + message(" ... and ${rest} more") + endif() +endfunction() diff --git a/tests/multi-arch/x86/entry_probe.cpp b/tests/multi-arch/x86/entry_probe.cpp index f55719106..f4278d6ad 100644 --- a/tests/multi-arch/x86/entry_probe.cpp +++ b/tests/multi-arch/x86/entry_probe.cpp @@ -95,7 +95,7 @@ float entry_all() { int main(int argc, char** argv) { if (argc == 2 && std::strcmp(argv[1], "--report") == 0) { - // Read by cmake/dispatch-checks/check-dispatch-execution.cmake, which + // Read by tests/multi-arch/dispatch-checks/check-dispatch-execution.cmake, which // breaks on this extent's kernels to see which level the call enters. std::printf("expect-level %d\n", svs_test::expected_level()); std::printf("probe-extent %zu\n", entry_report_dim); From 88ebd2688a849c15324065ae23300f2b3e800304 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 04:29:22 -0700 Subject: [PATCH 12/23] refactor(dispatch): parse the ISA level table in one place Route the hand-rolled ISA level and TU spec splits through centralized parsers that validate field counts, so malformed rows now fail where they are written instead of as empty variables later. The validator now shares the parser with the other two users, and its malformed-row fixture was updated to the shared message. Co-Authored-By: Claude Opus 5 --- cmake/dispatch-levels.cmake | 49 +++++++++++++++++++ cmake/generate-dispatch-surface.cmake | 7 ++- cmake/multi-arch.cmake | 6 +-- cmake/validate-dispatch-surface.cmake | 14 ++---- .../invalid-level-missing-field.cmake | 2 +- 5 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 cmake/dispatch-levels.cmake diff --git a/cmake/dispatch-levels.cmake b/cmake/dispatch-levels.cmake new file mode 100644 index 000000000..5b20ef57b --- /dev/null +++ b/cmake/dispatch-levels.cmake @@ -0,0 +1,49 @@ +# Copyright 2026 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. + +include_guard(GLOBAL) + +# Parses an ISA level spec from SVS_ISA_LEVELS and returns the three fields. +# The spec format is ||. +function(svs_parse_isa_level spec out_level out_arch out_infix) + string(REPLACE "|" ";" fields "${spec}") + list(LENGTH fields n) + if(NOT n EQUAL 3) + message(FATAL_ERROR "Malformed ISA level '${spec}': expected ||.") + endif() + list(GET fields 0 v0) + list(GET fields 1 v1) + list(GET fields 2 v2) + set(${out_level} "${v0}" PARENT_SCOPE) + set(${out_arch} "${v1}" PARENT_SCOPE) + set(${out_infix} "${v2}" PARENT_SCOPE) +endfunction() + +# Parses a translation unit spec from SVS_DISPATCH_TU_SPECS and returns the four fields. +# The spec format is |||. +function(svs_parse_tu_spec spec out_src out_level out_arch out_infix) + string(REPLACE "|" ";" fields "${spec}") + list(LENGTH fields n) + if(NOT n EQUAL 4) + message(FATAL_ERROR "Malformed TU spec '${spec}': expected |||.") + endif() + list(GET fields 0 v0) + list(GET fields 1 v1) + list(GET fields 2 v2) + list(GET fields 3 v3) + set(${out_src} "${v0}" PARENT_SCOPE) + set(${out_level} "${v1}" PARENT_SCOPE) + set(${out_arch} "${v2}" PARENT_SCOPE) + set(${out_infix} "${v3}" PARENT_SCOPE) +endfunction() diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake index 75a06c691..7d75b572f 100644 --- a/cmake/generate-dispatch-surface.cmake +++ b/cmake/generate-dispatch-surface.cmake @@ -22,6 +22,8 @@ include_guard(GLOBAL) +include("${CMAKE_CURRENT_LIST_DIR}/dispatch-levels.cmake") + set(SVS_DEFAULT_DISPATCH_SURFACE_FILE "${CMAKE_CURRENT_LIST_DIR}/dispatch-surface.cmake") set(SVS_DISPATCH_SURFACE_FILE "${SVS_DEFAULT_DISPATCH_SURFACE_FILE}" CACHE FILEPATH @@ -71,10 +73,7 @@ set(SVS_GEN_LEVEL_LOOP "\\\n") set(SVS_GEN_LEVEL_DEFINES "") set(SVS_DISPATCH_TU_SPECS) 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) + svs_parse_isa_level("${level_spec}" level arch infix) string(APPEND SVS_GEN_LEVEL_LOOP " M(${level}) \\\n") string(APPEND SVS_GEN_LEVEL_DEFINES "#define SVS_ISA_LEVEL_${level} 1\n") diff --git a/cmake/multi-arch.cmake b/cmake/multi-arch.cmake index e10e6e6c5..7d14d0802 100644 --- a/cmake/multi-arch.cmake +++ b/cmake/multi-arch.cmake @@ -16,14 +16,12 @@ # SVS_DISPATCH_TU_SPECS -- "|||", one entry per ISA # level. The extent list and the levels themselves are declared in # cmake/dispatch-surface.cmake. +include("${CMAKE_CURRENT_LIST_DIR}/dispatch-levels.cmake") include("${CMAKE_CURRENT_LIST_DIR}/generate-dispatch-surface.cmake") set(SVS_X86_OBJECT_FILES) foreach(tu_spec IN LISTS SVS_DISPATCH_TU_SPECS) - string(REPLACE "|" ";" tu_fields "${tu_spec}") - list(GET tu_fields 0 src) - list(GET tu_fields 2 arch) - list(GET tu_fields 3 infix) + svs_parse_tu_spec("${tu_spec}" src level arch infix) # Carries the instruction budget for this level, and nothing else. set(lib_name "svs_x86_${infix}") diff --git a/cmake/validate-dispatch-surface.cmake b/cmake/validate-dispatch-surface.cmake index 7bf1f21a6..edffe610a 100644 --- a/cmake/validate-dispatch-surface.cmake +++ b/cmake/validate-dispatch-surface.cmake @@ -40,6 +40,8 @@ cmake_policy(PUSH) cmake_policy(SET CMP0007 NEW) cmake_policy(SET CMP0057 NEW) +include("${CMAKE_CURRENT_LIST_DIR}/dispatch-levels.cmake") + if(NOT SVS_DISPATCH_SURFACE_FILE) message(FATAL_ERROR "SVS_DISPATCH_SURFACE_FILE is not set.") endif() @@ -107,17 +109,7 @@ 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) + svs_parse_isa_level("${level_spec}" level arch infix) foreach(field level arch infix) if(NOT ${field}) message(FATAL_ERROR diff --git a/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake b/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake index 2f71f45f6..03092a9f0 100644 --- a/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake +++ b/tests/cmake/dispatch-surface/invalid-level-missing-field.cmake @@ -13,6 +13,6 @@ # limitations under the License. # The TU infix is absent. -# EXPECT-ERROR: expected exactly three '|'-separated fields +# EXPECT-ERROR: Malformed ISA level 'AVX2|haswell': expected ||. set(SVS_SUPPORTED_DIMS 128) set(SVS_ISA_LEVELS "AVX2|haswell") From ed22dc13bc34d0939d50c92c60ce202fc09d0788 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 04:45:14 -0700 Subject: [PATCH 13/23] fix(dispatch): reject two ISA levels sharing one -march budget Two levels cannot share one -march instruction budget because the compiler then emits instructions from the weaker level at that budget, giving hosts routed to the weaker level instructions its runtime predicate does not guarantee. The instruction checker cannot catch this because it looks the budget up by -march and would check the weaker level against the (stronger) budget row, which forbids nothing. Co-Authored-By: Claude Opus 5 --- cmake/validate-dispatch-surface.cmake | 10 ++++++++ .../invalid-duplicate-arch.cmake | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/cmake/dispatch-surface/invalid-duplicate-arch.cmake diff --git a/cmake/validate-dispatch-surface.cmake b/cmake/validate-dispatch-surface.cmake index edffe610a..d05acb40c 100644 --- a/cmake/validate-dispatch-surface.cmake +++ b/cmake/validate-dispatch-surface.cmake @@ -108,6 +108,7 @@ endif() set(svs_seen_levels) set(svs_seen_infixes) +set(svs_seen_archs) foreach(level_spec IN LISTS SVS_ISA_LEVELS) svs_parse_isa_level("${level_spec}" level arch infix) foreach(field level arch infix) @@ -126,6 +127,14 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) "generated files and must be unique." ) endif() + if(arch IN_LIST svs_seen_archs) + message(FATAL_ERROR + "Duplicate -march '${arch}' in SVS_ISA_LEVELS; each level's -march " + "is its instruction budget, so two levels sharing one budget compile " + "the weaker level with instructions its runtime predicate does not " + "guarantee, and hosts routed to it fault." + ) + endif() if(NOT EXISTS "${SVS_X86_SRC_DIR}/${infix}.cpp") message(FATAL_ERROR "ISA level '${level}' has no translation unit: expected " @@ -135,6 +144,7 @@ foreach(level_spec IN LISTS SVS_ISA_LEVELS) endif() list(APPEND svs_seen_levels ${level}) list(APPEND svs_seen_infixes ${infix}) + list(APPEND svs_seen_archs ${arch}) endforeach() cmake_policy(POP) diff --git a/tests/cmake/dispatch-surface/invalid-duplicate-arch.cmake b/tests/cmake/dispatch-surface/invalid-duplicate-arch.cmake new file mode 100644 index 000000000..92ba4a3e5 --- /dev/null +++ b/tests/cmake/dispatch-surface/invalid-duplicate-arch.cmake @@ -0,0 +1,23 @@ +# Copyright 2026 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. + +# Each level's -march is its instruction budget. Two levels cannot share one +# -march: the weaker level is compiled with instructions its runtime predicate +# does not guarantee, and hosts routed to it fault. +# EXPECT-ERROR: Duplicate -march +set(SVS_SUPPORTED_DIMS 128) +set(SVS_ISA_LEVELS + "AVX2|haswell|avx2" + "AVX512|haswell|avx512" +) From 5cd6abcc41d3addfff8a8d53f4bf7c24fcf8086c Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 04:49:57 -0700 Subject: [PATCH 14/23] fix(dispatch): make the ISA level parser independent of caller policy A CMake function captures the policy stack where it is defined, so the parser's behaviour depended on which caller included it first. Under CMP0007 OLD an empty field vanishes when the spec is split, which turns a row with an empty middle field into a malformed-row error and silently invalidates the premise of the invalid-level-empty-field fixture. Setting the policy in the parser's own file makes the two error paths stable regardless of the caller. Co-Authored-By: Claude Opus 5 --- cmake/dispatch-levels.cmake | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmake/dispatch-levels.cmake b/cmake/dispatch-levels.cmake index 5b20ef57b..bc2da39cc 100644 --- a/cmake/dispatch-levels.cmake +++ b/cmake/dispatch-levels.cmake @@ -14,6 +14,11 @@ include_guard(GLOBAL) +## A function captures the policy stack where it is defined, so CMP0007 is set here rather +## than by the caller: under OLD an empty field vanishes and a row with one is called malformed. +cmake_policy(PUSH) +cmake_policy(SET CMP0007 NEW) + # Parses an ISA level spec from SVS_ISA_LEVELS and returns the three fields. # The spec format is ||. function(svs_parse_isa_level spec out_level out_arch out_infix) @@ -47,3 +52,5 @@ function(svs_parse_tu_spec spec out_src out_level out_arch out_infix) set(${out_arch} "${v2}" PARENT_SCOPE) set(${out_infix} "${v3}" PARENT_SCOPE) endfunction() + +cmake_policy(POP) From ff6858b9310a3a2e2210bb2b5e9ef813dddee9e8 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 04:50:16 -0700 Subject: [PATCH 15/23] docs(dispatch): explain why the surface checks are not Catch2 cases Every other test under tests/ is a Catch2 case, so a reader finding no Catch2 macros here has no way to tell whether that is deliberate. It is for the probes, whose tests are the link step and a gdb session, and provisional for the four cmake -P checkers. Co-Authored-By: Claude Opus 5 --- tests/multi-arch/dispatch-checks/README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/multi-arch/dispatch-checks/README.md b/tests/multi-arch/dispatch-checks/README.md index 331e913f6..1a5a48160 100644 --- a/tests/multi-arch/dispatch-checks/README.md +++ b/tests/multi-arch/dispatch-checks/README.md @@ -25,6 +25,27 @@ a new level. These tests are only added to the build when the x86 object libraries exist. Run `ctest` from `/tests`, not from the build root. +## Why these are not Catch2 cases + +Everything else under `tests/` is a Catch2 case registered with ctest by +`catch_discover_tests`. These are registered with `add_test` instead, and the +two probes under `../x86/` contain no Catch2 macros. That is deliberate for the +probes and provisional for the checkers. + +`link_probe.cpp` cannot be a Catch2 case because *its test is the link step*: it +names every kernel the surface declares and nothing else, so a declared but +uninstantiated kernel is an undefined symbol and the probe fails to link. Folding +it into the single `tests` binary would turn a surface bug into a whole-suite link +failure, and would stop `nm --undefined-only` from isolating the surface's symbols +from the hundred other objects. `entry_probe.cpp` is driven under gdb, which needs +its own small executable with predictable breakpoints. + +The four `cmake -P` checkers have no such excuse. They are scripts because they +shell out to `nm` and `objdump` and were easiest to write that way. Porting them +to Catch2 cases that shell out to the same tools is planned separately; it would +get real data structures and unit-testable helpers without adding a Python +dependency to the C++ build. + ## check-dispatch-linkage.cmake **Test:** `dispatch_surface_linkage` From 882b7618209dc5231848b9bc5e997f208a445fac Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 04:54:38 -0700 Subject: [PATCH 16/23] refactor(dispatch): read the surface from a generated manifest The generator emits dispatch_surface.manifest.cmake with the extent list and the ISA level enumerators, and the declaration check reads that instead of including the declaration itself. The check now depends on the surface's contents rather than on that file's format, location, or what else it happens to set. The AVX_AVAILABILITY read from distance_core.h stays: it is what derives the mangled enum digit rather than assuming it. Co-Authored-By: Claude Opus 5 --- cmake/generate-dispatch-surface.cmake | 20 +++++++++++ tests/multi-arch/CMakeLists.txt | 9 +++-- .../check-dispatch-declaration.cmake | 36 ++++++++----------- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/cmake/generate-dispatch-surface.cmake b/cmake/generate-dispatch-surface.cmake index 7d75b572f..09dbe11be 100644 --- a/cmake/generate-dispatch-surface.cmake +++ b/cmake/generate-dispatch-surface.cmake @@ -127,6 +127,26 @@ if(svs_surface_is_default) ) endif() +##### +##### Emit the manifest +##### + +# The ctest checks read this rather than including the declaration, so they depend on the +# surface's contents and not on that file's format, its location, or what else it sets. +set(svs_manifest_levels) +foreach(level_spec IN LISTS SVS_ISA_LEVELS) + svs_parse_isa_level("${level_spec}" svs_manifest_level _ _) + list(APPEND svs_manifest_levels "${svs_manifest_level}") +endforeach() +string(REPLACE ";" " " svs_manifest_levels_text "${svs_manifest_levels}") +string(REPLACE ";" " " svs_manifest_extents_text "${SVS_SUPPORTED_DIMS}") +file(GENERATE + OUTPUT "${CMAKE_BINARY_DIR}/dispatch_surface.manifest.cmake" + CONTENT "set(SVS_MANIFEST_FIXED_EXTENTS ${svs_manifest_extents_text}) +set(SVS_MANIFEST_LEVELS ${svs_manifest_levels_text}) +" +) + ##### ##### Report the surface ##### diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index 15e79c1bd..7b40ecbeb 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -23,6 +23,8 @@ ##### directory to its own project, where that points somewhere else entirely. ##### +include("${CMAKE_CURRENT_LIST_DIR}/../../cmake/dispatch-levels.cmake") + set(svs_dispatch_cmake_dir "${CMAKE_CURRENT_LIST_DIR}/dispatch-checks") # The object libraries exist so that each probe's single object file can be named @@ -87,7 +89,7 @@ if(svs_nm) NAME dispatch_surface_declaration COMMAND "${CMAKE_COMMAND}" - "-DSVS_SURFACE_FILE=${SVS_DISPATCH_SURFACE_FILE}" + "-DSVS_MANIFEST=${CMAKE_BINARY_DIR}/dispatch_surface.manifest.cmake" "-DSVS_TYPE_PAIR_HEADER=${CMAKE_CURRENT_LIST_DIR}/../../include/svs/multi-arch/x86/preprocessor.h" "-DSVS_ENUM_HEADER=${CMAKE_CURRENT_LIST_DIR}/../../include/svs/core/distance/distance_core.h" "-DSVS_ARCHIVE=$" @@ -104,10 +106,7 @@ if(svs_objdump) # runtime predicate does not guarantee faults on a host the dispatcher routes # to it, which no symbol-table check can see. foreach(tu_spec IN LISTS SVS_DISPATCH_TU_SPECS) - string(REPLACE "|" ";" tu_fields "${tu_spec}") - list(GET tu_fields 1 svs_level) - list(GET tu_fields 2 svs_arch) - list(GET tu_fields 3 svs_infix) + svs_parse_tu_spec("${tu_spec}" svs_src svs_level svs_arch svs_infix) add_test( NAME dispatch_instructions_${svs_infix} COMMAND diff --git a/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake b/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake index 929342945..021dd3609 100644 --- a/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake +++ b/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake @@ -18,7 +18,7 @@ ##### ##### Run in script mode: ##### -##### cmake -DSVS_SURFACE_FILE=cmake/dispatch-surface.cmake \ +##### cmake -DSVS_MANIFEST=build/dispatch_surface.manifest.cmake \ ##### -DSVS_TYPE_PAIR_HEADER=include/svs/multi-arch/x86/preprocessor.h \ ##### -DSVS_ENUM_HEADER=include/svs/core/distance/distance_core.h \ ##### -DSVS_ARCHIVE= \ @@ -30,9 +30,9 @@ ##### against a probe, and both are generated from the same header: a generator ##### that dropped an extent would drop it from both and still agree. The ##### expectation here is derived from the three hand-written sources instead -- -##### the extent list and levels, the type pairs, and the enumerator order -- so -##### the generated header is not consulted at all and a generator bug has nowhere -##### to hide. +##### the extent list and levels from the manifest, the type pairs, and the enumerator +##### order -- so the generated header is not consulted at all and a generator bug +##### has nowhere to hide. ##### # Without it, CMP0057 is unset in script mode and the `IN_LIST` tests below are @@ -41,32 +41,26 @@ cmake_minimum_required(VERSION 3.21) include("${CMAKE_CURRENT_LIST_DIR}/lib.cmake") -svs_require(SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER SVS_ARCHIVE SVS_CONSUMER_OBJECT SVS_NM) -svs_require_files(SVS_SURFACE_FILE SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER SVS_ARCHIVE SVS_CONSUMER_OBJECT) +svs_require(SVS_MANIFEST SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER SVS_ARCHIVE SVS_CONSUMER_OBJECT SVS_NM) +svs_require_files(SVS_MANIFEST SVS_TYPE_PAIR_HEADER SVS_ENUM_HEADER SVS_ARCHIVE SVS_CONSUMER_OBJECT) ##### ##### Ground truth 1: the extents and the ISA levels ##### -include("${SVS_SURFACE_FILE}") +include("${SVS_MANIFEST}") -if(NOT SVS_SUPPORTED_DIMS OR NOT SVS_ISA_LEVELS) +if(NOT SVS_MANIFEST_FIXED_EXTENTS OR NOT SVS_MANIFEST_LEVELS) message(FATAL_ERROR - "${SVS_SURFACE_FILE} declares no extents or no ISA levels; it is not a " - "dispatch-surface declaration." + "${SVS_MANIFEST} is not a dispatch-surface manifest; it does not declare " + "extents or ISA levels." ) endif() # svs::Dynamic mangles as its numeric value: the extent is a size_t template # argument, and Dynamic is SIZE_MAX. -set(svs_expected_extents ${SVS_SUPPORTED_DIMS} "18446744073709551615") - -set(svs_expected_levels) -foreach(level_spec IN LISTS SVS_ISA_LEVELS) - string(REPLACE "|" ";" fields "${level_spec}") - list(GET fields 0 level) - list(APPEND svs_expected_levels "${level}") -endforeach() +set(svs_expected_extents ${SVS_MANIFEST_FIXED_EXTENTS} "18446744073709551615") +set(svs_expected_levels ${SVS_MANIFEST_LEVELS}) ##### ##### Ground truth 2: the enumerator order, which fixes the mangled digit @@ -231,7 +225,7 @@ function(svs_compare prefix what) list(LENGTH wrong n_wrong) set(${prefix}_ERRORS 0 PARENT_SCOPE) if(NOT n_wrong EQUAL 0) - message("${n_wrong} of ${what} disagree with ${SVS_SURFACE_FILE}.") + message("${n_wrong} of ${what} disagree with the manifest.") message("Each entry is __. A count of 0 means") message("the declaration asks for kernels that were never built; a count") message("below the number of type pairs means the level is missing some.") @@ -248,7 +242,7 @@ math(EXPR errors "${ARCHIVE_ERRORS} + ${CONSUMER_ERRORS}") if(ARCHIVE_FOREIGN) list(LENGTH ARCHIVE_FOREIGN n) message("${n} kernels in the archive are outside the declared surface.") - message("Nothing in ${SVS_SURFACE_FILE} asks for them, so no consumer reaches") + message("Nothing in the manifest asks for them, so no consumer reaches") message("them and they only add to the size of the library.") message(" Outside the surface:") svs_report_first(ARCHIVE_FOREIGN 10) @@ -291,7 +285,7 @@ endforeach() string(REPLACE ";" ", " pair_display "${pair_display}") message( "dispatch declaration: ${svs_expected_total} kernels required by " - "${SVS_SURFACE_FILE} (${n_levels} levels x ${n_extents} extents x " + "the manifest (${n_levels} levels x ${n_extents} extents x " "${n_distances} distances x type pairs per level {${pair_display}}); the " "archive defines exactly those and the entry points reach all of them" ) From c87b4feb57600b2b873fc02f156c27fbf9438ff8 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 06:05:46 -0700 Subject: [PATCH 17/23] refactor(dispatch): name the probe executables after their sources The targets were dispatch_surface_probe and dispatch_entry_probe while the sources are x86/link_probe.cpp and x86/entry_probe.cpp, so neither binary could be found from the name of the file that produced it. The ctest names keep the dispatch_ prefix -- dispatch_link_probe, dispatch_entry_probe -- because `ctest -R dispatch` is how CI and the docs select this group, and renaming the tests to match the binaries would drop two of the eight out of that filter. The object libraries follow from the target name via svs_add_dispatch_probe, so the symbol-table checks pick up link_probe_objects and entry_probe_objects without further change. Co-Authored-By: Claude Opus 5 --- tests/multi-arch/CMakeLists.txt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index 7b40ecbeb..96af1e9aa 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -46,16 +46,16 @@ endfunction() # Names every kernel the surface declares and nothing else, so a declared-but- # uninstantiated kernel is an undefined symbol here. -svs_add_dispatch_probe(dispatch_surface_probe x86/link_probe.cpp) +svs_add_dispatch_probe(link_probe x86/link_probe.cpp) # Reaches the kernels through the entry points instead, so it also sees whether the # entry points route to the whole surface. -svs_add_dispatch_probe(dispatch_entry_probe x86/entry_probe.cpp) +svs_add_dispatch_probe(entry_probe x86/entry_probe.cpp) # 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) -add_test(NAME dispatch_entry_probe COMMAND dispatch_entry_probe) +add_test(NAME dispatch_link_probe COMMAND link_probe) +add_test(NAME dispatch_entry_probe COMMAND entry_probe) if(DEFINED CMAKE_NM AND CMAKE_NM) set(svs_nm "${CMAKE_NM}") @@ -76,7 +76,7 @@ if(svs_nm) NAME dispatch_surface_linkage COMMAND "${CMAKE_COMMAND}" - "-DSVS_PROBE_OBJECT=$" + "-DSVS_PROBE_OBJECT=$" "-DSVS_ARCHIVE=$" "-DSVS_NM=${svs_nm}" -P "${svs_dispatch_cmake_dir}/check-dispatch-linkage.cmake" @@ -93,7 +93,7 @@ if(svs_nm) "-DSVS_TYPE_PAIR_HEADER=${CMAKE_CURRENT_LIST_DIR}/../../include/svs/multi-arch/x86/preprocessor.h" "-DSVS_ENUM_HEADER=${CMAKE_CURRENT_LIST_DIR}/../../include/svs/core/distance/distance_core.h" "-DSVS_ARCHIVE=$" - "-DSVS_CONSUMER_OBJECT=$" + "-DSVS_CONSUMER_OBJECT=$" "-DSVS_NM=${svs_nm}" -P "${svs_dispatch_cmake_dir}/check-dispatch-declaration.cmake" ) @@ -129,7 +129,7 @@ if(svs_nm AND svs_gdb) NAME dispatch_surface_execution COMMAND "${CMAKE_COMMAND}" - "-DSVS_PROBE=$" + "-DSVS_PROBE=$" "-DSVS_NM=${svs_nm}" "-DSVS_GDB=${svs_gdb}" -P "${svs_dispatch_cmake_dir}/check-dispatch-execution.cmake" From fdc0b58001b6426a9f19f1bac7e9c37ffdfe65a8 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 06:23:07 -0700 Subject: [PATCH 18/23] test(dispatch): report how many kernels each probe called The probes printed only their accumulated distance sum, which says nothing about how many kernels ran: a macro list that expanded to fewer calls than the surface declares still produced a plausible float. The count makes that visible, and it cross-checks against a figure derived from a different source -- the declaration checker independently computes 918 kernels, which is what link_probe now reports. entry_probe reports 432 rather than 918 because each entry point picks one ISA level at runtime, so it reaches nine extents by sixteen type pairs by three distances, not the whole surface. Its count comes from a named constant next to entry_one, since that function's body is what fixes the calls per expansion. The comments explaining that printing defeats dead-code elimination are gone from both files: the printed message now says it. Co-Authored-By: Claude Opus 5 --- tests/multi-arch/x86/entry_probe.cpp | 30 ++++++++++++++++++++++------ tests/multi-arch/x86/link_probe.cpp | 29 +++++++++++++++++++++------ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/tests/multi-arch/x86/entry_probe.cpp b/tests/multi-arch/x86/entry_probe.cpp index f4278d6ad..2dff90be1 100644 --- a/tests/multi-arch/x86/entry_probe.cpp +++ b/tests/multi-arch/x86/entry_probe.cpp @@ -64,6 +64,13 @@ template const E* buffer() { return values.data(); } +// Both values are returned together so that the counter cannot drift from the sum it +// describes; the count is what distinguishes a reached entry point from one never emitted. +struct ProbeResult { + float total = 0; + std::size_t kernels = 0; +}; + // A template rather than a macro body: the svs::Dynamic entry points take a // length, and `if constexpr` only discards the other branch inside a template. template float entry_one() { @@ -79,13 +86,19 @@ template float entry_one() { } } -#define SVS_ENTRY_ONE(Ea, Eb, N) total += entry_one(); +// entry_one reaches L2, IP and cosine; a change to its body must change this constant. +constexpr std::size_t entry_kernels_per_call = 3; + +#define SVS_ENTRY_ONE(Ea, Eb, N) \ + total += entry_one(); \ + count += entry_kernels_per_call; #define SVS_ENTRY_DIM(N) SVS_FOR_EACH_TYPE_PAIR(SVS_ENTRY_ONE, N) -float entry_all() { +ProbeResult entry_all() { float total = 0; + std::size_t count = 0; SVS_FOR_EACH_SUPPORTED_DIM(SVS_ENTRY_DIM) - return total; + return ProbeResult{total, count}; } #undef SVS_ENTRY_DIM @@ -108,8 +121,13 @@ int main(int argc, char** argv) { return 0; } - // Printing keeps the calls from being optimized away, and makes the run a - // smoke test of every entry point over the whole surface. - std::printf("dispatch surface entry probe: %f\n", static_cast(entry_all())); + const ProbeResult result = entry_all(); + std::printf( + "dispatch surface entry probe: called %zu kernels that computed a total distance " + "sum of %f (total distance only printed here to avoid calls from getting optimized " + "away)\n", + result.kernels, + static_cast(result.total) + ); return 0; } diff --git a/tests/multi-arch/x86/link_probe.cpp b/tests/multi-arch/x86/link_probe.cpp index ea540a92d..8eda20974 100644 --- a/tests/multi-arch/x86/link_probe.cpp +++ b/tests/multi-arch/x86/link_probe.cpp @@ -78,13 +78,16 @@ template svs::lib::MaybeStatic probe_length() { total += svs::distance::L2Impl::compute( \ buffer(), buffer(), probe_length() \ ); \ + ++count; \ total += svs::distance::IPImpl::compute( \ buffer(), buffer(), probe_length() \ ); \ + ++count; \ total += \ svs::distance::CosineSimilarityImpl::compute( \ buffer(), buffer(), 1.0F, probe_length() \ - ); + ); \ + ++count; // The level's own type-pair list, not the full one: naming a pair the level has no // kernel for would instantiate the generic template right here. @@ -93,10 +96,19 @@ template svs::lib::MaybeStatic probe_length() { SVS_TYPE_PAIRS_FOR(LEVEL, SVS_PROBE_ONE, N, LEVEL) \ } -float probe_all() { +// Both values are returned together so that the counter cannot drift from the sum it +// describes; the count is what makes a link-only failure distinguishable from a call that +// was never emitted. +struct ProbeResult { float total = 0; + std::size_t kernels = 0; +}; + +ProbeResult probe_all() { + float total = 0; + std::size_t count = 0; SVS_FOR_EACH_DISPATCH_TARGET(SVS_PROBE_TARGET) - return total; + return ProbeResult{total, count}; } #undef SVS_PROBE_TARGET @@ -105,8 +117,13 @@ float probe_all() { } // 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())); + const ProbeResult result = probe_all(); + std::printf( + "dispatch surface probe: called %zu kernels that computed a total distance sum " + "of %f (total distance only printed here to avoid calls from getting optimized " + "away)\n", + result.kernels, + static_cast(result.total) + ); return 0; } From 669358eabb12d4df3b2e3aa5341aaff3728d2c01 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 07:34:18 -0700 Subject: [PATCH 19/23] test(dispatch): run the entry probe's --report mode under ctest The flag was only reached through the execution check's script, so a ctest run never showed the three lines it parses and a probe that stopped honouring the flag surfaced as a missing level rather than as itself. The mode returns before the kernel sweep, so this is a second invocation rather than an argument on the existing one, which keeps the kernel count visible too. --- tests/multi-arch/CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index 96af1e9aa..a64516885 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -57,6 +57,15 @@ svs_add_dispatch_probe(entry_probe x86/entry_probe.cpp) add_test(NAME dispatch_link_probe COMMAND link_probe) add_test(NAME dispatch_entry_probe COMMAND entry_probe) +# The probe's modes are disjoint: --report returns after the one call the execution check +# breaks on, so covering both under ctest takes two invocations. +add_test(NAME dispatch_entry_report COMMAND entry_probe --report) +# Exit status alone would pass on a probe that stopped honouring the flag, which the +# execution check would then report as a missing level rather than as the cause. +set_tests_properties( + dispatch_entry_report PROPERTIES PASS_REGULAR_EXPRESSION "expect-level [0-9]+" +) + if(DEFINED CMAKE_NM AND CMAKE_NM) set(svs_nm "${CMAKE_NM}") else() From 581bf2544d591b9ee77caf236d6011db129c7a6d Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 07:40:00 -0700 Subject: [PATCH 20/23] revert: "test(dispatch): run the entry probe's --report mode under ctest" This reverts 669358ea. The flag is reached again only through the execution check's script, so a ctest run shows the kernel counts but not the three lines that check parses. --- tests/multi-arch/CMakeLists.txt | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index a64516885..96af1e9aa 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -57,15 +57,6 @@ svs_add_dispatch_probe(entry_probe x86/entry_probe.cpp) add_test(NAME dispatch_link_probe COMMAND link_probe) add_test(NAME dispatch_entry_probe COMMAND entry_probe) -# The probe's modes are disjoint: --report returns after the one call the execution check -# breaks on, so covering both under ctest takes two invocations. -add_test(NAME dispatch_entry_report COMMAND entry_probe --report) -# Exit status alone would pass on a probe that stopped honouring the flag, which the -# execution check would then report as a missing level rather than as the cause. -set_tests_properties( - dispatch_entry_report PROPERTIES PASS_REGULAR_EXPRESSION "expect-level [0-9]+" -) - if(DEFINED CMAKE_NM AND CMAKE_NM) set(svs_nm "${CMAKE_NM}") else() From 925656e69ee4e5775a49c8fd9c01c334a2cca55d Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 11:30:19 -0700 Subject: [PATCH 21/23] docs(dispatch): correct the object-target naming rationale The comment justified naming the object target after the ISA level by asserting that more than one level can share an instruction budget. That stopped being true when validate-dispatch-surface.cmake began rejecting duplicate -march values as a hard error, so the stated reason no longer holds. Record the constraint that does apply: the target name is unique only because duplicate infixes are rejected too. Co-Authored-By: Claude Opus 5 --- cmake/multi-arch.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/multi-arch.cmake b/cmake/multi-arch.cmake index 7d14d0802..ca4812cd4 100644 --- a/cmake/multi-arch.cmake +++ b/cmake/multi-arch.cmake @@ -28,8 +28,8 @@ foreach(tu_spec IN LISTS SVS_DISPATCH_TU_SPECS) add_library(${lib_name} INTERFACE) target_compile_options(${lib_name} INTERFACE -march=${arch} -mtune=${arch}) - # Named after the level, not the -march: more than one level can share an - # instruction budget, and it is the level that says what is inside. + # Unique only because validate-dispatch-surface.cmake rejects duplicate + # infixes; two levels sharing one would collide on this target name. set(obj_name ${infix}_obj) add_library(${obj_name} OBJECT ${src}) target_link_libraries( From b30a325ef681966d0ebac9327f33d8a2c00ea07c Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 11:30:19 -0700 Subject: [PATCH 22/23] fix(dispatch): name the manifest in the unknown-ISA-level error SVS_SURFACE_FILE was never set anywhere in the tree, so the diagnostic rendered as "ISA level 'X' is declared in but is not an AVX_AVAILABILITY enumerator" -- the file context the message exists to give was always blank. The script is driven by SVS_MANIFEST. Co-Authored-By: Claude Opus 5 --- .../multi-arch/dispatch-checks/check-dispatch-declaration.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake b/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake index 021dd3609..7f67daca8 100644 --- a/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake +++ b/tests/multi-arch/dispatch-checks/check-dispatch-declaration.cmake @@ -88,7 +88,7 @@ set(svs_expected_digits) foreach(level IN LISTS svs_expected_levels) if(NOT DEFINED svs_digit_of_${level}) message(FATAL_ERROR - "ISA level '${level}' is declared in ${SVS_SURFACE_FILE} but is not an " + "ISA level '${level}' is declared in ${SVS_MANIFEST} but is not an " "AVX_AVAILABILITY enumerator." ) endif() From facfeb4f4bf0bb6eb587254a4d472305a5a0beee Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Wed, 2 Sep 2026 11:30:20 -0700 Subject: [PATCH 23/23] fix(dispatch): say nm or gdb when skipping the execution check The guard requires both tools, but the skip message named only gdb, so a host missing nm reported a cause that was not the one that fired. Co-Authored-By: Claude Opus 5 --- tests/multi-arch/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/multi-arch/CMakeLists.txt b/tests/multi-arch/CMakeLists.txt index 96af1e9aa..9614b24ea 100644 --- a/tests/multi-arch/CMakeLists.txt +++ b/tests/multi-arch/CMakeLists.txt @@ -135,5 +135,5 @@ if(svs_nm AND svs_gdb) -P "${svs_dispatch_cmake_dir}/check-dispatch-execution.cmake" ) else() - message(STATUS "gdb not found; skipping the dispatch surface execution test") + message(STATUS "nm or gdb not found; skipping the dispatch surface execution test") endif()