fix: CMake arch auto-detection SIGILL, typos, and improved error messages#137
Open
redbasecap-buiss wants to merge 5 commits intoalibaba:mainfrom
Open
fix: CMake arch auto-detection SIGILL, typos, and improved error messages#137redbasecap-buiss wants to merge 5 commits intoalibaba:mainfrom
redbasecap-buiss wants to merge 5 commits intoalibaba:mainfrom
Conversation
added 5 commits
February 16, 2026 22:40
…ported arch The auto-detection in _detect_x86_best() iterated from the highest microarchitecture (graniterapids) down and picked the first one the *compiler* accepted. Since compilers accept all -march= flags regardless of the host CPU, this would emit e.g. AVX-512BW instructions on machines that only support AVX2, causing SIGILL at runtime. Replace with -march=native which targets the actual build machine's ISA. Fixes alibaba#128 Fixes alibaba#92
The method check_need_adjuct_ctx() was consistently misspelled across both dense and sparse HNSW implementations. Renamed to check_need_adjust_ctx() for correctness.
- 'wiht' → 'with' in index_reducer.h, index.cc, mixed_streamer_reducer - 'spase' → 'sparse' in hnsw_builder_entity.h, hnsw_sparse_builder_entity.h - 'seperate' → 'separate' in vector_column_indexer.h - Fix extra space in README.md HTML align attribute
When using cosine metric without a quantizer on INT8 data, the error message was a bare 'Unsupported data type: ' with no detail. Now it explains that cosine without quantizer only supports FP32/FP16 and suggests using QuantizerType::kInt8 as a workaround. Related to alibaba#78
- Replace '!' (U+FF01) with '!' in cpu_features.h/.cc comments - Fix 'Orignal' → 'Original' in cosine_converter.cc error message
|
Ocean seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
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
This PR fixes several bugs and code quality issues:
🐛 Bug Fix: SIGILL on x86_64 CPUs without AVX-512 (Fixes #128, Fixes #92)
Root cause:
_detect_x86_best()incmake/option.cmakeiterated from the highest microarchitecture (graniterapids) downward and picked the first one the compiler accepted. Since compilers accept all-march=flags regardless of the host CPU, building on a machine with a modern compiler but an AVX2-only CPU would emit AVX-512BW instructions, causingSIGILLat runtime.Fix: Replace the flawed probe loop with
-march=native, which targets the actual build machine's instruction set.📝 Typo Fixes
check_need_adjuct_ctx→check_need_adjust_ctx— misspelled method name across 6 files in both dense and sparse HNSW implementationswiht→withinindex_reducer.h,index.cc,mixed_streamer_reducer.*spase→sparseinhnsw_builder_entity.h,hnsw_sparse_builder_entity.hseperate→separateinvector_column_indexer.hOrignal→Originalincosine_converter.ccerror message!→!incpu_features.h/.ccDoxygen commentsalign=" center"→align="center"💬 Improved Error Message (Related to #78)
When using cosine metric with INT8 data without a quantizer, the error was a bare
"Unsupported data type: "with no actionable detail. Now explains that cosine without quantizer only supports FP32/FP16 and suggests usingQuantizerType::kInt8.Changes
cmake/option.cmake-march=auto-detection to use-march=nativeadjuct→adjustsrc/core/interface/index.ccsrc/ailego/internal/cpu_features.*README.md