Conversation
added 2 commits
September 14, 2026 21:12
The GNU/Clang branch treated "not ARM" as "is x86", so every non-x86 ISA (loongarch64, riscv64, ppc64le, mips64, ...) was handed -mfpmath=sse and failed to compile any translation unit with "unrecognized command-line option '-mfpmath=sse'". The flag is already a no-op on x86-64 - SSE-based IEEE float math is the ABI default there and x87 excess precision cannot occur - so restricting it to x86 changes no behavior on existing targets while making the rest build.
The Eigen backend only implements the NHWC float32 path and hard-errors on NCHW (eigenbackend.cpp:2425) and on FP16 (eigenbackend.cpp:2663). Nothing compensated for that: TestCommon::overrideForBackends only knew about OpenCL and TensorRT, and testsearchmisc.cpp never called it at all. So every NN test invoked with NCHW or FP16 args - which is how runsearchtestslimited.sh and runsearchtests.sh invoke them - aborted with SIGABRT instead of running. Clamp in the two startNNEval helpers instead of extending overrideForBackends: they are the single choke point all NN tests go through, which also covers testsearchmisc.cpp's runNNSymmetriesTest and runNNBatchingTest. Guarded by USE_EIGEN_BACKEND, so other backends are unaffected.
This branch has not been deployed
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.
Support non-x86 builds, and let the NN tests run on the Eigen backend
Two independent fixes found while building KataGo on loongarch64 with the Eigen backend.
Each is in its own commit. Neither changes behavior on any currently supported platform.
1. Only pass
-mfpmath=sseon x86The GNU/Clang branch of the build setup treated "not ARM" as "is x86":
-mfpmath=sseonly exists on x86. On loongarch64 (and riscv64, ppc64le, mips64, ...) everytranslation unit fails immediately:
so the build dies at ~2% with no usable binary. The
else()is now an explicit x86 match.This is safe rather than merely cosmetic: the comment already notes that SSE-based IEEE float
math is the ABI default on x86-64 and that x87 excess precision cannot occur, so the flag is a
no-op there. Nothing loses IEEE compliance by restricting it to x86 — the goal was just to state
the requirement explicitly.
2. Normalize NHWC and FP16 test args for the Eigen backend
The Eigen backend only implements the NHWC float32 path and hard-errors otherwise:
eigenbackend.cpp:2425—inputsUseNHWC = false unsupportedeigenbackend.cpp:2663(and siblings) —useFP16 = true not supportedBut the test scripts pass exactly those.
runsearchtestslimited.shandrunsearchtests.shwerewritten for OpenCL, so they invoke e.g.
runsearchtests <model> false false 0 false(NCHW) andrunsearchtestsv8 <model> true true true(FP16). Every NN test therefore aborted with SIGABRTbefore doing any work.
TestCommon::overrideForBackends()exists for precisely this kind of normalization, but it onlyknows OpenCL and TensorRT, and
testsearchmisc.cpp'srunNNSymmetriesTest/runNNBatchingTestnever call it at all. Rather than extend that function (it takes only two params, so it cannot see
useFP16) and fix up ~13 call sites, the clamp goes in the twostartNNEvalhelpers:cpp/tests/testsearchcommon.cppcpp/tests/testtrainingwrite.cpp(its own file-localstartNNEval)Those are the single choke point every NN test goes through, so this also covers the two
testsearchmisc.cppcases that never reachedoverrideForBackends. Guarded by#if defined(USE_EIGEN_BACKEND), so OpenCL / CUDA / TensorRT / ROCm / Metal / ONNX builds arebit-for-bit unaffected.