[MOD-17925] SIMD dispatch: declarative ISA manifest, with the SQ8_FP32 pilot - #1023
Draft
dor-forer wants to merge 12 commits into
Draft
[MOD-17925] SIMD dispatch: declarative ISA manifest, with the SQ8_FP32 pilot#1023dor-forer wants to merge 12 commits into
dor-forer wants to merge 12 commits into
Conversation
The AVX512F_BW_VL_VNNI tier is compiled with -mavx512vl, but 6 of 12 runtime predicate sites omitted the avx512vl check, while 6 included it. A CPU with avx512f, avx512bw, and avx512vnni but without avx512vl would therefore be handed a function pointer into a TU the compiler was licensed to emit VL-encoded instructions in. Add avx512vl to all 12 sites, making them consistent.
The spaces/CMakeLists.txt correctly uses CMAKE_SYSTEM_PROCESSOR for the target machine, but tests/unit/CMakeLists.txt and tests/benchmark/CMakeLists.txt used CMAKE_HOST_SYSTEM_PROCESSOR, which breaks cross-compilation: a build machine with AVX512 targeting ARM would incorrectly pull in x86-specific instruction flags. Switch both to CMAKE_SYSTEM_PROCESSOR.
getCpuOptimizationFeatures() let a caller-supplied arch_opt override replace the detected CPU features outright, so a caller could claim a feature the real hardware lacks and be handed a function pointer into a SIMD tier that executes an illegal instruction. The override is a test-only affordance: no production caller passes a non-null arch_opt (every chooser in spaces.cpp uses the nullptr default), tests use it to force a specific tier. Every existing test only ever clears bits (232 clears in test_spaces.cpp, zero sets), so intersecting the override with the detected features instead of replacing them is a no-op for the whole existing suite while closing the hole for any future or malformed test. The new intersectWithDetectedFeatures() helper performs a byte-wise AND over the two structs' object representation rather than hand-listing fields, since cpu_features::X86Features and cpu_features::Aarch64Features are composed entirely of `int <name> : 1;` bitfields with no other members: a byte-wise AND therefore performs a per-feature logical AND, and hand-writing per-field assignments would rot the moment cpu_features adds a field. A static_assert guards the trivially-copyable assumption the memcpy-based AND relies on. The new CpuFeatureIntersectionX86 / CpuFeatureIntersectionAarch64 tests in test_spaces.cpp enforce the pure-bitfield layout assumption itself: they walk every enum value via cpu_features' own GetX86FeaturesEnumValue / GetAarch64FeaturesEnumValue readers and compare against the field-wise AND, so a future non-bitfield member would make the byte-wise AND diverge and fail loudly. They also cover the two cases called out by the task: clearing one feature out of an all-detected mask leaves every other feature unchanged, and claiming a feature the detected set lacks still comes out clear.
VectorSimilaritySpaces_no_optimization (L2/L2.cpp, IP/IP.cpp) is the scalar fallback that runtime dispatch degrades to, and the "no architecture flag" property that makes it safe on every deployment CPU was only a comment. The root CMakeLists.txt repeatedly appends to whatever CMAKE_CXX_FLAGS it inherits, and consumers export CXXFLAGS into this build (RediSearch's build.sh does), so an inherited -march=native could silently vectorize the fallback itself with instructions the target CPU may not have, defeating runtime dispatch in the one place that must be universally safe. Pin a conservative baseline (-march=x86-64 or -march=armv8-a, guarded by CHECK_CXX_COMPILER_FLAG, empty on other architectures) and apply it with target_compile_options(... PRIVATE ...) to VectorSimilaritySpaces_no_optimization and VectorSimilaritySpaces. The target-level flag cannot clobber the per-tier ISA flags that set_source_files_properties() attaches to each functions/*.cpp, because CMake emits source-level COMPILE_FLAGS after target-level COMPILE_OPTIONS on the same command line, and gcc does not let a later -march= disable an earlier -m feature flag either way. Reject at configure time any -march=native, -mtune=native, -mcpu=native, or -march=/-mcpu= set to something other than the chosen baseline, found in CMAKE_CXX_FLAGS, CMAKE_CXX_FLAGS_<BUILD_TYPE>, or the CXXFLAGS environment variable. A non-native -mtune value is deliberately still allowed: it only changes instruction scheduling, not which instructions are legal, so rejecting it would break legitimate consumer builds for no safety gain. Add a small, separate CI job that injects -march=native through CXXFLAGS and asserts the configure fails; it proves a negative, so it stays out of the main build jobs. The job is dispatched through event-pull_request.yml like the other build jobs, instead of triggering on pull_request directly. Its job id is listed in pr-validation's needs, so a failure there blocks the pull request like every other required check.
Downgrade the ISA value-mismatch case in _spaces_reject_isa_override (src/VecSim/spaces/CMakeLists.txt) from FATAL_ERROR to a warning. The target-level baseline applied further down with target_compile_options() already keeps the scalar fallback (VectorSimilaritySpaces_no_optimization) on the conservative baseline regardless of what a consumer sets, so the fatal error was never the actual protection mechanism. Refusing to configure on a merely different, explicit, non-native -march/-mcpu (for example -march=x86-64-v2, a distro default, or -mcpu=neoverse-n1) broke consumers declaring a legitimate deployment floor, and RediSearch exports CXXFLAGS into this build. The =native case keeps FATAL_ERROR: there is no safe way to honor an auto-detected baseline. De-duplicate the CMAKE_SYSTEM_PROCESSOR regex matching into a single SPACES_ARCH_FAMILY variable, computed once, with the baseline computation and both tier-gating if() blocks branching on it instead of re-matching the regex. Fix the spaces-isa-baseline workflow to grep for "Refusing to configure" instead of "-march=native": the job asserts a negative (configure must fail), and grepping for the same string it injected as CXXFLAGS would let an unrelated configure failure that happens to echo the flag pass as if it were the expected rejection. The retained FATAL_ERROR message still contains that exact phrase. Add GetCpuOptimizationFeaturesAppliesMaskX86 and GetCpuOptimizationFeaturesAppliesMaskAarch64 to tests/unit/test_spaces.cpp. The existing CpuFeatureIntersection* tests only exercise intersectWithDetectedFeatures() directly; they never call getCpuOptimizationFeatures(&mask), which is the actual entry point every real caller goes through and the function whose behavior the mask-intersection fix changed. The new tests build an all-0xFF claiming mask, deliberately not derived from the detected set, so they cannot pass vacuously.
Two ways the scalar fallback could still end up compiled above the deployment baseline, both found in review and both measured on hardware. The baseline probe failed open. CHECK_CXX_COMPILER_FLAG compiles its test with CMAKE_CXX_FLAGS included and treats a warning that names a flag as failure, so an inherited -mcpu=neoverse-n1 makes gcc warn that the switch conflicts with -march=armv8-a and the probe reports Failed. On x86_64 and aarch64 the baseline flag is supported by every compiler that can build this library, so a failed probe never means the flag is unsupported: it means the inherited flags broke the probe. Continuing there dropped the baseline from the scalar fallback while the configure still succeeded, which is the hazard the guard exists to prevent. It now fails closed and names the likely cause. Verified on a Neoverse-N1. Explicit ISA feature flags bypassed the guard entirely. It only matched -march and -mcpu, and a later -march= baseline does not cancel a feature flag: measured with gcc 13, "-mavx2 ... -march=x86-64" still emitted 317 and 161 AVX-family instructions into the two scalar fallback objects. Those flags cannot be neutralised reliably, so they are refused. Disabling forms (-mno-...) and -mtune= stay allowed, since neither raises the set of legal instructions. The guard's CI job asserted only the -march=native case, so it caught neither of these. It is now a matrix over the native variants and the feature flags, plus a positive case checking that an explicit non-native baseline still configures and still warns. Also gate the 22 single-line SQ8 VNNI test guards on avx512vl, so the tests mirror the production predicate this branch tightened. The multi-line INT8 and UINT8 guards already required it.
`FP16_L2Sqr` and `FP16_InnerProduct` widen each stored half with FP16_to_FP32 and accumulate into a `float`. Four SIMD tiers did not: AVX512FP16 kept `__m512h sum` and reduced into a `_Float16`, NEON kept `float16x8_t acc` with `vfmaq_f16`, and SVE kept `svfloat16_t acc` with `svmla_f16_x`. On any CPU that selects one of those tiers the whole vector was summed in an 11-bit mantissa, so the same function returned a different answer depending on the machine. Two consequences, both silent. Precision: simulating both accumulation orders over 20,000 random 128-dimension fp16 vectors gives a maximum relative error of 5.6e-3 for the fp16 accumulator against 1.9e-7 for the fp32 one, so nearest neighbours and their ordering change. Overflow: 65504 is the largest finite fp16 value, so 32 elements of 200.0, all ordinary fp16 values, drive a half precision accumulator past it and the result becomes infinity. All four tiers now widen and accumulate in fp32. The x86 kernels mirror L2_AVX512F_FP16.h and IP_AVX512F_FP16.h step for step, since after widening there is nothing half-precision-specific left to do differently; they also gain the second accumulator that #984 added to the sibling fp16 tiers and not to these. NEON and SVE keep their existing four-way unrolling, with each accumulator becoming a pair covering the lower and upper halves of a register. The unit tests could not have caught this. Both fp16 baselines accumulated the reference in `_Float16` too, so the test compared a half precision kernel against a half precision reference and passed within its 1% tolerance whatever the kernel did. They now accumulate in `float` via FP16_to_FP32, mirroring the scalar functions exactly. That alone is still not a regression test: the randomized cases draw values from [-0.99, 0.99], where a half precision accumulator's worst error over dim 32..256 is about 0.54%, inside the 1% budget. FP16SpacesTest.LargeValuesDoNotOverflowTheAccumulator closes that gap with inputs whose expected totals are exact in fp32 and infinite in fp16, and it calls the public choosers so whichever tier the running CPU selects is tested. The ARM half of this change depends on the NEON_HP/NEON_FHM translation unit split from #1018. Widening to fp32 and then issuing vfmaq_f32 is exactly the pattern gcc contracts into FMLAL, so in a translation unit compiled with +fp16fml the fix would emit FMLAL into the plain half-precision path and fault on any core without FEAT_FHM. Measured on gcc 12: the NEON kernels compile to 4 FMLAL at -march=armv8.2-a+fp16fml and 0 at +fp16. With #1018 the HP tier is compiled at +fp16 only, and NEON_HP.cpp.o contains no FMLAL. Verified on an AWS Graviton2 (Neoverse-N1, asimdhp without asimdfhm, gcc 12), which executes the NEON path: the full spaces suite passes 1529/1529 with this change on top of #1019, where the same change on a main base fails 106 tests with SIGILL. On x86 (Ice Lake, gcc 13) the suite passes 1569/1569, and the AVX512FP16 kernels compile with -mavx512fp16 -Werror leaving no fmadd*ph, subph or mulph. The AVX512FP16 tier itself needs Sapphire Rapids or later to execute and the SVE tier needs an SVE core, so neither runs on the hardware available here; both are covered by CI.
Two declarative tables that will become the single source of truth for the SIMD dispatch layer. Nothing consumes them yet: this commit only states, in one place and in one vocabulary, what is currently spread across cmake/*InstructionFlags.cmake, spaces/CMakeLists.txt, and about 40 hand-written chooser bodies. isa_features.def has one row per CPU feature: the token tiers name, its architecture, the compiler flag fragment it contributes, the lowest -march level that admits that fragment, and the cpu_features field naming the runtime bit. Three relations are spelled distinctly rather than as one "implies", because they answer different questions: ARCH_IMPLIES enumerates which CPU masks are possible, FLAG_ENABLES records what a fragment forces the compiler to assume, and LEVEL_REQUIRES what raising -march forces. isa_tiers.def has one row per tier, with FLAGS_FROM and GUARANTEES as separate columns. That separation is the point of the whole design: a flag fragment expands to a compiler-defined bundle, so compiling with a flag does not tell you which bits the deployment CPU must have. Two live bugs came from conflating them, and both are now visible as a column difference instead of being invisible. Every row was derived by reading the build rather than by inference, and three things that reading corrected: - The tier set is 23, matching the 23 set_source_files_properties() entries. An earlier count of 20 came from grepping OPT_* in the two dispatch files, which misses tiers guarded by another tier's macro. - AVX2_F16C, AVX2_FMA_F16C and SSE4_F16C are tiers in their own right, not variants of AVX2, AVX2_FMA and SSE4. Each is its own translation unit with its own flags and predicate, carrying the SQ8-to-FP16 kernels that need vcvtph2ps, while the plain tiers carry the SQ8-to-FP32 kernels that do not. Legacy code expressed this as one shared OPT_F16C guard around three differently predicated branches, which is why they look like variants. - The F16C and SSE4_F16C predicates include bits beyond the tier's nominal feature: f16c && fma3 && avx and sse4_1 && f16c && avx respectively. -mf16c is VEX-encoded, so the flag puts AVX-state instructions in reach and the predicates already account for it. Priorities are spaced by 10 so a tier can be inserted without renumbering, and they reproduce the legacy selection order for both SQ8_FP32 (VNNI, AVX2_FMA, AVX2, SSE4) and SQ8_FP16 (AVX512F, AVX2_FMA_F16C, AVX2_F16C, SSE4_F16C). A parity test will pin that rather than leaving it asserted here.
vecsim_manifest.cmake reads both .def files into CMake variables and derives a tier's compile flag string from its FLAGS_FROM column alone. x86 unions the -m fragments in token order; ARM emits one -march at the highest level among the tier's tokens, then each distinct +fragment, because ARM -march strings are monolithic rather than a union of independent options. GUARANTEES is parsed and carried through untouched, never consulted here. The runtime predicate is not derived from flags, because a fragment expands to a compiler-defined bundle that differs between compilers for the same -march string. Keeping the derivation one-directional is what stops this module from quietly recreating the unsound design. Verified: the derived flags are byte-for-byte identical to the 23 hardcoded strings in spaces/CMakeLists.txt today, compared by extracting both and diffing. That equality is the whole claim behind calling the manifest a source of truth, so it is worth stating as a measurement rather than an intention. Nothing consumes the derivation yet, so this commit cannot change what the build emits. Validation is deliberately fatal rather than skip-on-error: a tier naming an undefined token, a tier naming a token of the other architecture, an unknown ARM level, or a manifest that parses to zero rows all stop the configure. Each of those would otherwise disable tiers silently while the configure still succeeded, which is the failure mode this whole series has been about. tier_probe.cmake is carried over unchanged from the held P0.3 work: it try_compiles a tier's real source under the tier's real flags with CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY, since these translation units have no main() and a default executable probe would fail to link for every tier and disable the entire dispatch layer while reporting success. Two parser bugs found by running it rather than reading it. Commas are not CMake list separators, so the token columns arrived as single strings and foreach(IN LISTS) saw one element; they are now converted to semicolons. And the first version set CMP0057 to use IN_LIST, which leaks policy into the includer under CMP0011, so the membership tests use list(FIND) and the module needs no policy at all.
Adds the manifest layer next to the existing tier blocks rather than replacing them. The blocks above still compile the translation units; this block derives the same facts from the manifests, asserts the two agree, and exposes what the dispatch layer will need. The assertion is the reason both descriptions can coexist safely. For every tier the legacy blocks compiled, its COMPILE_FLAGS are read back off the source property the build will actually use and compared against the manifest's derivation, and a mismatch stops the configure. Nothing is copied by hand, so the two cannot drift silently while the migration is in progress. When the last legacy chooser goes, the legacy blocks go with it and the manifest is the only description left. Availability is VECSIM_BUILT_<TIER>, always defined as 0 or 1 rather than positive-only, so if constexpr sees a value for every tier on every build, including tiers belonging to the other architecture. The defines live on an internal INTERFACE target consumed by the library and the tests, not on the installed target: directory scope is how OPT_* silently vanished from tests/unit earlier in this series, and consumers should not see private build capabilities at all. Buildability comes from the whole-tier probe, which compiles the tier's real source under its real derived flags. A tier that fails is reported and excluded, never silently dropped. functions/all_tiers.h is generated with file(GENERATE) from the tiers that actually built. A TIER() expansion cannot emit an #include, and a hand-maintained list would need a checker to prove it matched the manifest; a generated one cannot disagree with its source. Measured on both machines. x86 (Ice Lake, gcc 13): configure clean, 15 legacy tiers built, 15 probes recorded in the cache keyed by compiler identity and flag string, 15 includes generated, all 23 defines emitted as 0 or 1, and zero flag-mismatch failures. ARM (Neoverse-N1, gcc 12): configure clean, 8 tiers, 8 includes, same 0-or-1 coverage, zero mismatches. Both gates were also shown to fail when they should, since a check that cannot fail proves nothing. Pointing a bogus tier at an existing stem tripped the equality assertion with the two flag strings named. Giving a bogus tier its own source and an unaccepted flag failed the probe instead: configure still exited 0, the tier was reported unavailable, VECSIM_BUILT_BOGUS_TIER came out 0, and it was absent from the umbrella while the other 15 includes remained.
tier_info.h turns both .def files into a Tier enumeration and a TierInfo<Tier> trait, by including the manifests with the row macros defined. It names no intrinsics, because it is compiled into every dispatching translation unit including the baseline ones. Two facts per tier come from two different columns and are kept apart: compiled comes from VECSIM_BUILT_<TIER>, which CMake sets from the whole-tier compile probe, and answers whether this build produced the tier's object. supported() is folded exclusively from GUARANTEES and answers whether the CPU in front of us may execute it. supported() never reads FLAGS_FROM, because a flag fragment expands to a compiler-defined bundle that differs between compilers for the same -march string, so a flags-derived predicate can claim support the hardware lacks. That is not an abstract concern, and the tests pin it on the real rows where the columns differ rather than on a synthetic tier. NEON_FHM has FLAGS_FROM (ASIMDFHM) and GUARANTEES (ASIMDHP, ASIMDFHM): with asimdfhm set and asimdhp clear, supported() returns false, where a flags-derived predicate would return true. That combination is the SIGILL reproduced on a Graviton2 earlier in this series. On x86 the same test covers SSE4_F16C, whose predicate carries avx because -mf16c is VEX-encoded, and the VNNI tier, whose predicate carries avx512vl because the tier is compiled with it. Measured on both machines: x86 (Ice Lake, gcc 13) 15 tiers, zero flag mismatches, spaces suite 1568/1568 and both TierInfo tests passing. ARM (Neoverse-N1, gcc 12) 8 tiers, zero mismatches, 1531/1531 and both passing. The spaces totals are exactly additive over the stack (1528 + 2 from #1019 + 1 from #1021 on ARM), so this commit adds no tests to that suite and moves none. Two implementation notes worth keeping: FeaturesType was a function-local alias inside getCpuOptimizationFeatures, so it is hoisted to namespace scope in spaces.h. Repeating the architecture selection in a second header is the duplication this refactor exists to remove. The predicate body is selected by the preprocessor from the tier's arch column, not by if constexpr. The discarded branch of an if constexpr in a non-template function still has to name things that exist, and an x86 build never declares the ARM feature accessors, so the if constexpr version did not compile. Every tier still gets a specialization on every build, so arch, priority and name stay visible to tests and tooling regardless of architecture.
kernel.h declares what a tier provides for one (metric, data type) pair. The primary template means "not provided", so a combination no tier implements is simply absent rather than needing a guard, and because dispatch will test `if constexpr (Kernel<...>::exists && TierInfo<T>::compiled)`, make() is never referenced for a combination that does not exist nor linked for a tier that did not compile. That is what keeps the two gates separate in the generated code: the toolchain decides what exists, the running CPU decides what is selected, and neither is allowed to stand in for the other. The fields are the ones that genuinely differ per combination, taken from the existing choosers rather than invented: residual_mod and residual_policy (FixedModulus for the CHOOSE_IMPLEMENTATION tiers, SveRuntime for the two that defer to the runtime vector length), align_mod and align_bytes, min_dim and max_dim. No priority override field is included, because no tier needs one and an unused field is configuration surface the pilot is supposed to be proving it does not need. Specializations are declared with a VECSIM_KERNEL macro so a tier header reads as a short table and a contributor adds a kernel by copying a neighbouring row. They live in the tier's declaration header; make() is defined in the tier's own translation unit, the only one carrying that tier's flags, and inside the region where the chooser macro is defined. Populated for the pilot family: SQ8_FP32 for L2, IP and Cosine across the seven tiers that implement it, 21 rows and 21 make() bodies. Every body is the legacy chooser's macro invocation copied unchanged rather than rewritten, so the kernel selected for a given dim cannot differ from today's. The trait values were read out of the choosers the same way: residual step 32 for the three AVX2-and-up tiers, 16 for SSE4 and NEON, runtime for SVE and SVE2; alignment 16/8/8/4 bytes on the four x86 tiers and none on ARM. One asymmetry is now expressed more precisely than the legacy code expressed it. The x86 choosers early-return the scalar path for dim < 8 with one arch-wide check, while the ARM side has no such gate. That becomes min_dim on the specialization: 8 on the four x86 tiers, 1 on the three ARM ones. Same behaviour, stated per tier instead of per architecture, so a future tier with a different floor does not have to fight an arch-wide constant. TypePolicy is left generic. SQ8_FP32 has neither a predicate gate nor a dimension ceiling, so specializing it here would add unused code; the BF16 endianness gate and the uint8 exactness bound belong to the families that have them. Measured on both machines: x86 15 tiers, 12 make() definitions across four tier objects, spaces 1568/1568; ARM 8 tiers, 9 across three, 1531/1531. Zero flag mismatches, TierInfo tests passing on both. Nothing dispatches through these yet, so behaviour is unchanged by construction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the changes in the pull request
The dispatch layer states the same facts in three places:
cmake/*InstructionFlags.cmakedecides which tiers the toolchain can build,spaces/CMakeLists.txtrestates each tier's flags, and about 40 chooser bodies restate each tier's runtime predicate, alignment rule and dimension gate behind stacked#ifdefs. This replaces that with two declarative manifests and, eventually, one genericdispatch<Metric, Type>().The whole design turns on keeping two gates independent, because this library is compiled on one machine and run on another:
try_compileof the tier's real source under its real flags. The build machine's own CPU is deliberately irrelevant; gating on it would ship binaries missing every tier the builder happened to lack.GUARANTEES.isa_tiers.deftherefore carriesFLAGS_FROMandGUARANTEESas two columns rather than one list. A flag fragment expands to a compiler-defined bundle that differs between gcc and clang for the same-marchstring, so a predicate derived from flags can claim support the hardware lacks. Where the columns differ, the difference is load-bearing and commented:NEON_FHMis compiled at+fp16fmlbut requiresasimdhp && asimdfhmat runtime, which is the SIGILL fixed in #1018;SSE4_F16Ccarriesavxbecause-mf16cis VEX-encoded.What is in this draft
spaces/isa_features.def, 23 features.spaces/isa_tiers.def, 23 tiers. Every row derived by reading the build, not inferred.cmake/vecsim_manifest.cmakeparses both and derives each tier's flags fromFLAGS_FROMalone. Verified byte-for-byte identical to all 23 hardcoded flag strings, twice: offline by diffing, and inside the configure by reading each tier'sCOMPILE_FLAGSback off the source property and asserting equality, so the two descriptions cannot drift while both exist.cmake/tier_probe.cmakegates buildability on the whole-tiertry_compile, withCMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARYbecause these TUs have nomain(). An excluded tier is always reported, never silently dropped.VECSIM_BUILT_<TIER>defined as 0 or 1 always, on an internal interface target consumed by the library and the tests, soif constexpris well formed for every tier and consumers see none of it.functions/all_tiers.his generated from the tiers that built.spaces/tier_info.h: theTierenum andTierInfo<Tier>, withsupported()folded exclusively fromGUARANTEES.spaces/kernel.h:Kernel<Metric, Type, Tier>andTypePolicy<Type>, populated for SQ8_FP32 across L2, IP and Cosine on all seven tiers that implement it. Eachmake()body is the legacy chooser's macro invocation copied unchanged, so the kernel selected for a givendimcannot differ.Still to come: the generic
dispatch<>()and repointing the three SQ8_FP32 choosers at it, a parity gate against the legacy choosers over the enumerated CPU masks, codegen equivalence per changed tier object, and LTO hardening.Which issues this PR fixes
Main objects this PR modified
src/VecSim/spaces/isa_features.defandisa_tiers.defcmake/vecsim_manifest.cmakeandcmake/tier_probe.cmakesrc/VecSim/spaces/CMakeLists.txt, the manifest layersrc/VecSim/spaces/tier_info.handkernel.hfunctions/<tier>.{h,cpp}tests/unit/test_spaces.cpp, twoTierInfocasesVerification
Measured after each commit on x86 (Ice Lake, gcc 13) and ARM (Neoverse-N1, gcc 12): 15 and 8 tiers built, zero flag mismatches, spaces suites 1568/1568 and 1531/1531, both
TierInfotests passing. Test counts are exactly additive over the stack, so these commits add nothing to that suite and move nothing in it.Both gates were also made to fail deliberately, since a check that cannot fail proves nothing. Pointing a bogus tier at an existing stem tripped the equality assertion, naming both flag strings. Giving a bogus tier its own source and an unaccepted flag failed the probe instead: configure still exited 0, the tier was reported unavailable, its define came out 0, and it was absent from the umbrella while the other 15 includes remained.
Review focus
The manifest format is the part worth arguing about now, while only one family depends on it. Specifically: whether
FLAGS_FROMandGUARANTEESas two columns reads better than the alternatives, whether the tier granularity is right (the three F16C combination TUs are modelled as first-class tiers, not variants ofAVX2/AVX2_FMA/SSE4), and whether a contributor adding a kernel would find the tier header's rows self-explanatory without reading the design doc.Mark if applicable