[MLAS] Fix MLAS AVX2 build failure on toolchains without AVX-VNNI assembler support - #28767
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes MLAS’s AVX-VNNI usage detection robust on toolchains where the assembler cannot assemble AVX-VNNI instructions (e.g., GCC 11 paired with older binutils), preventing AVX2 build failures caused by incorrectly enabling AVX-VNNI paths.
Changes:
- Replace a compiler-version heuristic in
cmake/onnxruntime_mlas.cmakewith an end-to-end compile probe to decide whether to add-mavxvnni. - Update several AVX2 SQNBitGEMM headers to gate AVX-VNNI code on
__AVXVNNI__instead of__GNUC__version checks.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmake/onnxruntime_mlas.cmake | Uses a compile test to detect usable -mavxvnni and conditionally applies AVX-VNNI compile flags. |
| onnxruntime/core/mlas/lib/sqnbitgemm_m1_sym_kernel_avx2_int8_blklen32.h | Switches AVX-VNNI preprocessor gating to __AVXVNNI__. |
| onnxruntime/core/mlas/lib/sqnbitgemm_kernel_avx2_int8_blklen64.h | Switches AVX-VNNI preprocessor gating to __AVXVNNI__. |
| onnxruntime/core/mlas/lib/sqnbitgemm_kernel_avx2_int8_blklen32.h | Switches AVX-VNNI preprocessor gating to __AVXVNNI__. |
| onnxruntime/core/mlas/lib/sqnbitgemm_kernel_avx2_int8_blklen16.h | Switches AVX-VNNI preprocessor gating to __AVXVNNI__. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This PR appears to address the compiler/assembler capability failure, but we have confirmed a related runtime issue that may remain when the AVX-VNNI probe succeeds. We build |
…remove debug messages
|
@copilot - Please resolve the conflict and address this comment - #28767 (comment) |
…-build-issues # Conflicts: # cmake/onnxruntime_mlas.cmake
Conflict resolved in the latest merge commit ( |
|
The latest capability-probe update fixes the compiler/assembler failure, but it When the probe succeeds, We reproduced this deterministically:
Changing the preprocessor guards to As a validated downstream mitigation, we set A durable upstream fix likely requires separate AVX2/AVX-VNNI translation units |
@copilot - Please address this |
|
@copilot - Please rebase with tip of main branch |
Merged the latest main (tip: |
|
@copilot - There are some breaking builds: This is one error that I spotted: FAILED: [code=1] libonnxruntime.so.1.29.0 Can you fix these issues ? |
… by adding inline to header definition
Fixed in the latest commit. The root cause: Fix: added |
tianleiwu
left a comment
There was a problem hiding this comment.
Thanks for the clean ISA-separation refactor — moving the AVX-VNNI kernels and dispatch table into a dedicated -mavxvnni TU while keeping the pure-AVX2 kernels in a flagless TU, and replacing the __GNUC__ > 10 heuristic with a check_cxx_source_compiles probe (__AVXVNNI__ guards), is the right structure. The two earlier threads (probe needing -mavx2; status-message -mf16c mismatch) look addressed.
One High-Priority build concern remains (inline on cmake/onnxruntime_mlas.cmake):
Undefined-reference link failure on the exact toolchains this PR targets. When the probe MLAS_COMPILER_SUPPORTS_AVXVNNI is false (RHEL 9 / GCC 11 / binutils < 2.36), the else() branch empties mlas_platform_srcs_avx2vnni, so sqnbitgemm_kernel_avx2vnni.cpp is dropped. That TU is now the only definition of MlasSQNBitGemmDispatchAvx2vnni (the prior definition in sqnbitgemm_kernel_avx2.cpp was removed here). But platform.cpp still takes its address unconditionally (this->QNBitGemmDispatch = &MlasSQNBitGemmDispatchAvx2vnni;), mlasi.h declares it extern const unconditionally, and test_sqnbitgemm_2bit_gemm.cpp references it in 6 places — none guarded by the probe result. Since the address is taken in reachable code, the linker needs the definition regardless of runtime CPUID, so the build fails with undefined reference to MlasSQNBitGemmDispatchAvx2vnni on precisely the old-binutils toolchains this PR is meant to unblock. CI won't catch it because CI toolchains support AVX-VNNI and compile the TU.
Suggested fix: always compile sqnbitgemm_kernel_avx2vnni.cpp; make only the -mavxvnni flag conditional. The __AVXVNNI__ header guards already compile the VNNI paths out when -mavxvnni is absent, so the dispatch table falls back to the correct AVX2 kernels and the symbol is always defined. Details inline.
…bit header AVXVNNI guards
|
Ran this branch (head 2057360) through the build matrix I have, since it is the same failure class as the probe issue in #28236 and it restructures files I have current changes near. On gcc 15.2 / binutils 2.46 (WSL2): the probe sets One behavior change worth a deliberate call: the new kernel guard is Also a sequencing note: #29886 modifies |
Description
On toolchains where the assembler lacks AVX-VNNI support (e.g., RHEL 9.x with GCC 11 + binutils < 2.36), MLAS AVX2 builds fail with
inlining failed ... target specific option mismatchandno such instruction: vpdpbusd.CMake (
cmake/onnxruntime_mlas.cmake)CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "11", which incorrectly includes GCC 11.x since"11.3.1" > "11.0.0") with acheck_cxx_source_compilesprobe that tests actual-mavxvnnisupport end-to-end (compiler + assembler).-mavxvnniis only added when the probe succeeds.Header files (
sqnbitgemm_kernel_avx2_int8_blklen{16,32,64}.h,sqnbitgemm_m1_sym_kernel_avx2_int8_blklen32.h)#if !defined(__GNUC__) || (__GNUC__ > 10)with#if !defined(__GNUC__) || defined(__AVXVNNI__).__AVXVNNI__is defined by GCC and Clang only when-mavxvnniis active, making this a reliable feature check rather than a version heuristic. Non-GCC compilers (MSVC) are unaffected since__GNUC__remains undefined for them.Motivation and Context
RHEL 9.x ships GCC 11 with binutils that predate AVX-VNNI assembler support. The version-based heuristics in cmake and the C++ guards both incorrectly enabled the AVX-VNNI code path for GCC 11, causing build failures. Red Hat currently carries these as downstream patches; this upstreams a robust fix.