Measure 20-byte Hamming codes eight at a time - #5587
Open
mnorris11 wants to merge 6 commits into
Open
Conversation
Contributor
|
@mnorris11 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D118971285. |
mnorris11
pushed a commit
to mnorris11/faiss
that referenced
this pull request
Sep 7, 2026
Summary: Pull Request resolved: facebookresearch#5587 A 20-byte code has no vector kernel at any x86 SIMD level. `HammingComputer20` measures one distance with three scalar popcounts at every level, including `AVX512_VPOPCNT`, so a scan of 160-bit codes runs the same code everywhere. Vectorizing a single 20-byte distance does not help. A masked 256-bit load plus `vpopcntq` plus a horizontal reduction measures the same as the scalar body, because the reduction costs more than the three popcounts it replaces. Measuring several codes per call does help, because it amortizes that reduction. This adds `hamming_batch()` to `HammingComputer20` at `AVX512_VPOPCNT`, which measures eight codes at once: - Eight codes span 160 bytes. `_mm512_popcnt_epi8` gives a count per byte. - `_mm512_sad_epu8` sums each aligned 8-byte group. - A 20-byte code covers two whole groups plus half of a group it shares with its neighbour, so each code needs one scalar popcount for its half. `IVFBinaryScannerL2::scan_codes` calls `hamming_batch()` when the computer declares a `batch_size`, and measures the remaining codes one at a time. A computer without one keeps the existing loop, so no other code size changes. The byte-wise popcount needs `AVX512_BITALG`, which is a separate CPUID bit from `AVX512_VPOPCNTDQ`. This adds the flag to the `AVX512_VPOPCNT` level and requires both bits to select it. Every CPU that has VPOPCNTDQ also has BITALG, so no CPU loses the level. An alternative byte-wise popcount built from `vpshufb`, which needs neither bit and would run at the plain `AVX512` level, measures only 1.08x against 1.54x, so it is not worth the wider reach. Differential Revision: D118971285
mnorris11
force-pushed
the
export-D118971285
branch
from
September 7, 2026 03:51
9b861f5 to
fcfc261
Compare
…ookresearch#5572) Summary: Pull Request resolved: facebookresearch#5572 **TL;DR:** Stops aarch64 running scalar code where a NEON or SVE kernel exists, adds the two missing NEON byte-domain kernels, and fixes `FAISS_SIMD_LEVEL` falling all the way to `NONE` on an uncompiled level. On aarch64 several faiss dispatch paths ran scalar code, or ran an `ARM_NEON` kernel where an `ARM_SVE` kernel already existed. This change enforces the order SVE -> NEON -> NONE on an ARM host. It also adds the one kernel pair that was missing. T287037898 reported the scalar-quantizer half. On aarch64 `IndexScalarQuantizer` with `QT_8bit_direct` or `QT_8bit_direct_signed` saved memory but lost throughput. `Refine(SQ8)` has the same problem, because `IndexRefine::search` calls `refine_index->get_distance_computer()`. ## What each call site gets | Call site | Before | After | | --- | --- | --- | | `QT_8bit_direct` on aarch64 | float-domain `DCTemplate` | NEON `DistanceComputerByte` | | `QT_8bit_direct_signed` on aarch64 | float-domain `DCTemplate` | NEON `DistanceComputerByteSigned` (new) | | `IndexFlat` distance computers, SVE host | `ARM_NEON` | `ARM_SVE` | | `AdditiveQuantizer::compute_centroid_norms`, SVE host | `ARM_NEON` | `ARM_SVE` | | `SuperKMeans` `block_l2`, SVE host | `ARM_NEON` | `ARM_SVE` | | `pq_code_distance` wrappers, SVE host | `ARM_NEON` | `ARM_SVE` | | `with_VectorDistance`, SVE host | `ARM_NEON` | `ARM_SVE` | | `FAISS_SIMD_LEVEL=ARM_SVE`, build without SVE | `NONE` | nearest compiled level | ## Three causes, fixed separately **1. Two missing kernels.** `sq-neon.cpp` held a scalar `DistanceComputerByte<Sim, ARM_NEON>` that nothing could reach, and no `DistanceComputerByteSigned<Sim, ARM_NEON>` at all. Both are now real NEON kernels. L2 and the unsigned inner product use `vabdq_u8`, `vmull_u8` and `vpadalq_u16`. The bias-encoded inner product uses `veorq_u8`, `vmull_s8` and `vpadalq_s16`. Both agree bit for bit with the AVX2 specializations, which the tests require. Two invariants deserve a note: - The L2 path keeps an unsigned accumulator. A `vmull_u8` square reaches 255^2 = 65025, which an int16 lane would read as negative. - For x in 0 to 255, `x ^ 0x80` read as `int8` is exactly `x - 128`. That is how the kernel removes the +128 bias before `vmull_s8`. **2. Dispatch chains that list only x86 levels.** The `if constexpr` chains in `sq-dispatch.h` enumerated x86 levels only, so an ARM host fell through to the float path. This adds `ARM_NEON` to four chains: two in `select_distance_computer_body`, and two in `sq_select_InvertedListScanner`. The chain in `is_dimension_compatible` already included ARM. This does not add `ARM_SVE`. The scalar-quantizer entry points dispatch with a mask that holds no `ARM_SVE` bit, so an SVE host already falls through to the `ARM_NEON` case and now gets these kernels. Adding `ARM_SVE` would instantiate the empty primary template in `distance_computers.h`. This corrects that template's stale comment. **3. Level masks that hide existing SVE kernels.** The default mask holds no `ARM_SVE` bit, so the dispatch fell through `case ARM_SVE` to `ARM_NEON`. This uses `with_simd_level_a1` at every site where a real SVE kernel exists and links: `IndexFlat.cpp` at four sites, `AdditiveQuantizer::compute_centroid_norms`, `block_l2` in `SuperKMeans.cpp`, all three wrappers in `pq_code_distance-generic.cpp`, and `with_VectorDistance` in `distances_dispatch.h`. The `IndexFlat` sites matter most. `faiss::fvec_L2sqr()` already used the SVE mask, so on an SVE host the free function ran SVE while `IndexFlatL2`'s distance computer ran NEON. Also: `FAISS_SIMD_LEVEL=ARM_SVE` on a build without SVE compiled in used to skip every level and run at `NONE`, because `with_selected_simd_levels` has no case label for an uncompiled level. It now walks down to the nearest compiled level. The override is still honoured when the CPU lacks the level, since forcing a level is the point of it. Only uncompiled levels are corrected. The unused `AVAILABLE_SIMD_LEVELS_A2` is deleted. It is the NEON-to-NONE trap in constant form, with no users. This change keeps the existing mask names. A follow-up renames them to say what they hold. ## Out of scope In rough order of remaining value: - `IndexPQ.cpp` and `IndexIVFPQ.cpp` still pin PQ to `ARM_NEON` on an SVE host. A mask change is not enough. `pq_code_distance-sve.cpp` includes only `pq_scan_impl.h`, and `with_HammingComputer<ARM_SVE>` has no complete type. - `distances_aarch64.cpp` forwards five `ARM_NEON` specializations to `<SIMDLevel::NONE>`, so a non-SVE aarch64 host runs scalar code in the IVFFlat scan. - `rabitq_neon.cpp` forwards all six `ARM_NEON` specializations to `<SIMDLevel::NONE>`. There is no SVE variant. - There is no `block_l2<ARM_NEON>` and no `exhaustive_L2sqr_blas_cmax<ARM_NEON>`. - Part two of T287037898, which is SDOT and SMMLA. `FEAT_DotProd` and `FEAT_I8MM` are not expressible as a `SIMDLevel`. The right shape is a runtime `getauxval(AT_HWCAP)` check inside the ARM translation unit, which follows the `SIMDConfig::avx512_split` precedent. Note the reporter's caveat: SMMLA shows no improvement until the scan loop is tiled, so the tiling must land in the same change. Differential Revision: D118682598
Summary: **TL;DR:** Renames the dispatch masks from `A0`/`A1`/`A2` to names that say which levels they hold; no behaviour change. The dispatch masks were named `A0`, `A1` and `A2`. The names say nothing about which levels a mask holds, so a caller had to read `simd_dispatch.h` to pick one. That is how several call sites ended up on a mask with no `ARM_SVE` bit while a dedicated SVE kernel existed. This renames every mask and helper after its contents. It changes no behaviour. | Old name | New name | Holds | | --- | --- | --- | | `AVAILABLE_SIMD_LEVELS_A0` | `..._BASE` | NONE, AVX2, AVX512, ARM_NEON, RISCV_RVV | | `AVAILABLE_SIMD_LEVELS_A0_SPR` | `..._BASE_WITH_SPR` | BASE + AVX512_SPR | | `AVAILABLE_SIMD_LEVELS_A1` | `..._BASE_WITH_SVE` | BASE + ARM_SVE | | (new) | `..._BASE_WITH_SPR_AND_SVE` | BASE + AVX512_SPR + ARM_SVE | The helpers follow: `with_simd_level_a0_spr` becomes `with_simd_level_with_spr`, and `with_simd_level_a1` becomes `with_simd_level_with_sve`. `BASE` is the right name for the default mask because `ARM_NEON` belongs to it. NEON is mandatory on aarch64, the way AVX2 is the x86 baseline, while `ARM_SVE` is optional. So the ARM fallback chain is SVE to NEON to NONE. The combined `BASE_WITH_SPR_AND_SVE` mask is new. No call site needs it yet, but the pair of optional levels is only expressible once the names say what they hold. Differential Revision: D119025991
Summary: ## What Split the existing VPOPCNTDQ RaBitQ and Hamming kernels from the full Sapphire Rapids SIMD level. DD builds now expose an `AVX512_VPOPCNT` capability for CPUs such as Ice Lake and Zen 4. The fallback chain is: `AVX512_SPR -> AVX512_VPOPCNT -> AVX512 -> AVX2 -> NONE` The ordinary static AVX512 target remains unchanged and contains no VPOPCNT instructions. ## Why The VPOPCNT kernels only require baseline AVX-512 plus `AVX512_VPOPCNTDQ`. Tying them to the SPR level unnecessarily excluded CPUs that support VPOPCNTDQ but not AVX512-FP16, notably Zen 4. ## Changes made while importing Two changes were needed on top of the pull request. **Feature detection read the wrong CPUID leaf.** `AVX512_VPOPCNTDQ` and `AVX512_VNNI` were read from leaf 1 ECX. Both live in leaf 7 subleaf 0 ECX, at bits 14 and 11. Leaf 1 ECX holds unrelated bits at those positions, so the new level was never selected on any CPU. Measured on an AMD Genoa host before the fix: the level reported as unavailable and detection returned `AVX512`. After the fix the same host reports `AVX512_VPOPCNT`. **The build definitions outside CMake were not updated.** The pull request renames three files. The other build description still named the old paths and had no entry for the new level, so the library did not build. This adds the level constant, its compiler flags, its `COMPILE_SIMD_AVX512_VPOPCNT` define, and the renamed paths. **The new level was not registered in the compiled-level mask.** `compiled_simd_levels()` reports which levels a binary holds, and `FAISS_SIMD_LEVEL` walks down from the requested level until it finds one that is present. The new level had no entry, so the override always stepped past it to `AVX512` and printed that the level was not compiled in. This was reported by `test_dispatch_with_env_var` on the open-source dynamic-dispatch build. **The test helper inferred available levels from `/proc/cpuinfo`.** A CPU feature does not imply that a level is compiled into the build, so the helper listed a level the override then refused. It now asks `SIMDConfig::is_simd_level_available()`, which reports compiled and supported. That is a subset of compiled, so every level it returns is one the override accepts. The mask and helper names also follow the naming used in the parent diff, so `AVAILABLE_SIMD_LEVELS_BASE_WITH_VPOPCNT` and `with_simd_level_with_vpopcnt`. Pull Request resolved: facebookresearch#5531 Test Plan: ``` buck2 test fbcode//mode/opt -c faiss.dynamic_dispatch=true \ fbcode//faiss/tests:test_simd_levels fbcode//faiss/tests:test_rabitq_simd ``` Result: all pass. The build failures the imported version reported are cleared. All three configurations build: ``` buck2 build fbcode//mode/dev fbcode//faiss:faiss fbcode//faiss:faiss_no_multithreading fbcode//faiss:faiss_omp_mock buck2 build fbcode//mode/opt -c faiss.dynamic_dispatch=true <same three targets> buck2 build fbcode//mode/dev -c fbcode.arch=aarch64 <same three targets> ``` Detection on an AMD Genoa host, which has AVX512_VPOPCNTDQ and AVX512_BF16 but not AVX512_FP16: | | before the CPUID fix | after | | --- | --- | --- | | detected level | `AVX512` | `AVX512_VPOPCNT` | | `AVX512_VPOPCNT` available | 0 | 1 | | `AVX512_SPR` available | 0 | 0 | Differential Revision: D118969754 Pulled By: mnorris11
Summary: **TL;DR:** Guards a `k` of 0 in binary `scan_codes`, which previously read past an empty heap, and documents that a caller bounds a top-k scan by seeding the heap with the radius. `IVFBinaryScannerL2::scan_codes` reads `simi[0]` to find the heap top. A k of 0 leaves no heap to read, so the scan read past the end of an empty array. This guards that case and returns 0. The change also records how a caller bounds a top-k scan by a radius, because nothing said so and the answer is not obvious from the code. The heap top is the only bound the scan applies. A caller that wants the k nearest codes inside a radius therefore seeds every heap slot with that radius, in place of the neutral value that `heap_heapify` writes. The scan then rejects any code at or beyond the radius. A slot the scan never fills keeps its label of -1, which is how the caller tells a result from an empty slot. That idiom needs no new state on the scanner, and it keeps one rule in the loop: a code must beat the heap top. A second bound would make the loop harder to reason about, and a radius has no meaning for a caller that only wants the k nearest codes. The loop now holds the heap top in a local. Only an accepted code can lower it, so the loop reads it again inside the branch rather than on every iteration. On a scan that accepts few codes, that turns a load into a register read for almost every code. Differential Revision: D118925150
Summary: **TL;DR:** Points `IndexBinaryIVF` at the VPOPCNT level and instantiates its scanner in the translation unit compiled with `-mavx512vpopcntdq`, worth 1.58x at 256-bit and 1.41x at 512-bit on Genoa. Two things kept the binary Hamming paths on scalar popcount, whatever the host. `IndexBinaryIVF` builds its scanner and runs its searches through `with_simd_level`, which uses the BASE level mask. That mask holds no `AVX512_VPOPCNT` bit, so on a host that reports that level the dispatch falls through to `AVX512`, where the wide computers use scalar popcount. Two build variants, `faiss_omp_mock` and `faiss_no_multithreading`, hardcoded dynamic dispatch off. They compiled the AVX2 kernels only, so a consumer of either variant could not use runtime dispatch at all, whatever the build setting said. This change: - Moves the three dispatch sites in `IndexBinaryIVF.cpp` to `with_simd_level_with_vpopcnt`. Below that level the dispatch falls through to `AVX512`, as before. - Instantiates `IndexBinaryIVF_impl.h` in `hamming_avx512_vpopcnt.cpp`. That is the one translation unit compiled with `-mavx512vpopcntdq`, which the VPOPCNT computers need. Without it the new dispatch has no symbol to call. The include must follow the computer specializations, so it is placed after them. - Lets both build variants honour the dynamic dispatch setting. `HammingComputer16` and `HammingComputer20` still inherit the scalar body at this level, so a 16-byte or 20-byte code gains nothing here. Differential Revision: D118874433
Summary: **TL;DR:** Adds a batched 20-byte Hamming kernel that measures eight codes per call, the only thing that helps the 160-bit width, worth 1.21x end to end. A 20-byte code has no vector kernel at any x86 SIMD level. `HammingComputer20` measures one distance with three scalar popcounts at every level, including `AVX512_VPOPCNT`, so a scan of 160-bit codes runs the same code everywhere. Vectorizing a single 20-byte distance does not help. A masked 256-bit load plus `vpopcntq` plus a horizontal reduction measures the same as the scalar body, because the reduction costs more than the three popcounts it replaces. Measuring several codes per call does help, because it amortizes that reduction. This adds `hamming_batch()` to `HammingComputer20` at `AVX512_VPOPCNT`, which measures eight codes at once: - Eight codes span 160 bytes. `_mm512_popcnt_epi8` gives a count per byte. - `_mm512_sad_epu8` sums each aligned 8-byte group. - A 20-byte code covers two whole groups plus half of a group it shares with its neighbour, so each code needs one scalar popcount for its half. `IVFBinaryScannerL2::scan_codes` calls `hamming_batch()` when the computer declares a `batch_size`, and measures the remaining codes one at a time. A computer without one keeps the existing loop, so no other code size changes. The byte-wise popcount needs `AVX512_BITALG`, which is a separate CPUID bit from `AVX512_VPOPCNTDQ`. This adds the flag to the `AVX512_VPOPCNT` level and requires both bits to select it. Every CPU that has VPOPCNTDQ also has BITALG, so no CPU loses the level. An alternative byte-wise popcount built from `vpshufb`, which needs neither bit and would run at the plain `AVX512` level, measures only 1.08x against 1.54x, so it is not worth the wider reach. Differential Revision: D118971285
mnorris11
force-pushed
the
export-D118971285
branch
from
September 7, 2026 15:13
fcfc261 to
8b2d73c
Compare
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
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.
Summary:
TL;DR: Adds a batched 20-byte Hamming kernel that measures eight codes per call, the only thing that helps the 160-bit width, worth 1.21x end to end.
A 20-byte code has no vector kernel at any x86 SIMD level.
HammingComputer20measures one distance with three scalar popcounts at every level, includingAVX512_VPOPCNT, so a scan of 160-bit codes runs the same code everywhere.Vectorizing a single 20-byte distance does not help. A masked 256-bit load plus
vpopcntqplus a horizontal reduction measures the same as the scalar body, because the reduction costs more than the three popcounts it replaces.Measuring several codes per call does help, because it amortizes that reduction. This adds
hamming_batch()toHammingComputer20atAVX512_VPOPCNT, which measures eight codes at once:_mm512_popcnt_epi8gives a count per byte._mm512_sad_epu8sums each aligned 8-byte group.IVFBinaryScannerL2::scan_codescallshamming_batch()when the computer declares abatch_size, and measures the remaining codes one at a time. A computer without one keeps the existing loop, so no other code size changes.The byte-wise popcount needs
AVX512_BITALG, which is a separate CPUID bit fromAVX512_VPOPCNTDQ. This adds the flag to theAVX512_VPOPCNTlevel and requires both bits to select it. Every CPU that has VPOPCNTDQ also has BITALG, so no CPU loses the level.An alternative byte-wise popcount built from
vpshufb, which needs neither bit and would run at the plainAVX512level, measures only 1.08x against 1.54x, so it is not worth the wider reach.Differential Revision: D118971285