Correct the architecture-specific code paths in the native math layer - #1283
Merged
Conversation
The 32-bit arm/powerpc branch computed a truncated single-word product and split that into halves, so a NATIVE_SIZE=64 build on those targets produced silently incorrect results. Replace the per-architecture fallbacks with one half-word decomposition, which also covers Emscripten and removes the #error for unlisted architectures. The aarch64 assembly is dropped: gcc and clang provide __int128 on aarch64, so it was unreachable.
… target -march=native is the x86 spelling; on aarch64 the preferred spelling is -mcpu=native, which selects the tuning model as well as the architecture. Current Apple clang accepts both, but other aarch64 toolchains accept only one and some accept neither, and cmake does not validate compiler flags at configure time, so a bad flag fails at the first compiled source file instead of at configuration. Probe the candidates and use the first the compiler accepts, warning instead of failing when none are. Also scope the clang lane-selection predicates to x86. The borrow-test conditional subtract and the inline Barrett form were chosen from x86 clang measurements, where the plain unsigned compare lowers to a mispredicting branch below AVX2; requiring __x86_64__ lets other targets take the plain forms. On aarch64 the two are equivalent (24 versus 23 instructions under Apple clang 21) and it has an unsigned 64-bit vector compare. Generated code on x86 is unchanged, verified by assembly diff under gcc-13, clang-18 and clang-18 -march=native.
- Probe -march=native/-mcpu=native separately for the C and C++ compilers, since the two frontends may accept different options, and apply each language's flag only when its own probe passes. - Reset WITH_NATIVEOPT to OFF when the C++ compiler accepts neither flag, so config_core.h, OpenFHEConfig.cmake, and the WITH_NATIVEOPT-gated code paths stay consistent with the flags actually applied. - Replace the hand-rolled uint128_t block in MultD with the bit-identical MultDPortable, which also gives the portable algorithm CI coverage by construction through the NATIVE_SIZE=128 jobs.
dsuponitskiy
approved these changes
Sep 4, 2026
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
2 participants
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.
Two fixes to hardware carveouts that had drifted out of correctness, found while
reviewing the native-integer layer for non-x86 targets.
Double-word multiply. The
__arm__/__powerpc__branch ofMultDsat inthe 64-bit block but computed a truncated single-word product and split that into
halves, so a default
NATIVE_SIZE=64build on 32-bit ARM or PowerPC producedsilently wrong results (2^32 * 2^32 returned 0). The per-architecture fallbacks
are replaced with one half-word decomposition that is correct everywhere, which
also covers Emscripten and removes the
#errorfor unlisted architectures. Theaarch64 inline assembly is dropped: gcc and clang provide
__int128on aarch64,so that block was unreachable, and the surviving path compiles to a single
umulhanyway.Machine-specific optimization flag.
WITH_NATIVEOPT=ONhardcoded-march=native, the x86 spelling. Since cmake does not validate compiler flagsat configure time, a toolchain that rejects it fails at the first compiled source
file rather than at configuration. The flag is now probed, preferring
-mcpu=nativeon aarch64 (it selects the tuning model too), with a warning and aportable build if neither spelling is accepted. The clang lane-selection
predicates are scoped to
__x86_64__at the same time, since they were chosenfrom x86 measurements and other targets should take the plain forms.