Fix-Point opened a new pull request, #21491: URL: https://github.com/apache/nuttx/pull/21491
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).* ## Summary This series fixes compile-time, correctness and portability problems in `include/nuttx/lib/math32.h`, introduces the standard 128-bit integer types, and adds an architecture hook for the 64x64 high multiply. - `arch/int128_t: add standard support for int128_t` Define `_int128_t`/`_uint128_t` and `__INT128_DEFINED` in the per-architecture `types.h` when the compiler provides `__int128`, and expose the `int128_t`/`uint128_t` typedefs from `<stdint.h>` (with a single-point fallback for architectures whose `types.h` does not define the base types). The local `uint128_t` typedef in `crypto/ecc.c` is guarded against collision. - `lib/math32: fix div64_const not compatible for pre-process macro` `div64_const()` was a braced-group statement expression and could not be used in static initializers (`boards/arm/cxd56xx/spresense/src/cxd56_automount.c` fails with "braced-group within expression allowed only inside a function"). It is now a static inline function. - `include/nuttx: fix compile error.` Convert `umul64_const()` and `div64_const32()` from macros to static inline functions: the old macros expanded `LOG2_FLOOR()` at every use site and produced enormous tokens which broke `-Wmisleading-indentation`. `umul64_const()` now returns the 64-bit result instead of assigning it through an output parameter. - `lib/math32: Fix coverity of divide 0.` Add `DEBUGASSERT(d > 1)` to `invdiv_init_param32()` and `invdiv_init_param64()` to cover the documented divisor contract. - `include/math32.h: use standard uint128_t for math32.h compatible` Use the `uint128_t` typedef from `<stdint.h>` instead of the compiler-specific `__uint128_t`, and switch the availability checks to `__INT128_DEFINED` so the software fallback is used whenever the architecture does not provide the 128-bit type. - `lib/math32: fix undefined shift in div64_const32` `p = 1 << LOG2_FLOOR(b)` shifted a signed `int` literal by up to 31 bits (undefined behavior). Use an unsigned literal and assert the divisor contract (`b > 1`) before the computation. - `include/stdint.h: provide int128_t/uint128_t fallback for all architectures` Define the base types in `<stdint.h>` when the architecture did not supply them but the compiler supports `__int128`, so the fast paths guarded by `__INT128_DEFINED` work uniformly on all architectures. - `lib/math32: make FLS() width-generic and 64-bit safe on all hosts` `FLS()` only examined the high 32 bits on 64-bit hosts and returned wrong results for inputs above 2^32 on 32-bit hosts (`FLS(1ULL << 40)` returned 0 instead of 41), silently breaking `LOG2_FLOOR()`/`LOG2_CEIL()` for 64-bit values. The bit scan is now layered by argument width (FLS32/FLS64/FLS128) and selected with `sizeof()`. - `lib/math32: widen div64_const() base to 64-bit` `div64_const()` took a `uint32_t` divisor and silently truncated any wider one: `div_const(n, 1ULL << 40)` passed 0 as the divisor and divided by zero at runtime. The parameter is now `uint64_t`; `div64_const32()` is only used when the divisor fits in 32 bits (explicitly narrowed), larger divisors fall back to plain division, and the divisor contract (`base > 0`) is asserted at entry. Widening is source compatible with existing callers. - `lib/math32: use plain division in div64_const() with CONFIG_DEBUG_NOOPT` The magic-number setup in `div64_const()` is only a win when it is folded away at compile time; unfolded per call it contains several divisions and is far slower than a plain division. Without optimization (`CONFIG_DEBUG_NOOPT`) the setup is not folded, so `div64_const()` is defined as a plain division there. - `lib/math32: extract invdiv_umulh64() as the up_umulh64() arch hook` The high half of a 64x64 product is exactly the UMULH primitive some architectures implement in a single instruction. Rename `invdiv_umulh64()` to the general `umulh64()` and let architectures provide `up_umulh64()` (following the `up_umul64_const()` precedent); the C implementation stays as the fallback. ## Impact - No functional change for existing users: the converted helpers are result-equivalent to the previous macro implementations (randomized differential tests), and `div_const*()` callers are unaffected. - `div64_const()`/`div_const*()` accept a 64-bit divisor (source compatible with existing callers) and can be used where a plain expression is required; without optimization they expand to a plain division. - `umul64_const()` returns its result instead of using an output parameter, and `div64_const32()` returns the quotient instead of assigning through its first argument (no callers outside math32.h). - `FLS()`/`LOG2_FLOOR()`/`LOG2_CEIL()` give correct results for 64-bit arguments on all hosts. - New standard types `int128_t`/`uint128_t` in `<stdint.h>` when the compiler provides `__int128`; new optional arch hook `up_umulh64()`. - Build process, hardware and configuration are unaffected; no new Kconfig options. ## Testing Build Host: Linux x86_64 (gcc 13.1.0, clang 14.0.0) Target(s): qemu-intel64:nsh (x86_64), QEMU/KVM, SMP: ``` sudo qemu-system-x86_64 -enable-kvm -cpu host,+invtsc,+vmware-cpuid-freq,kvmclock=off -smp 4 -m 512M -kernel nuttx -nographic -serial mon:stdio ``` Verified: - `tools/checkpatch.sh`: every commit and the full range pass all checks. - Unit tests (gcc and clang, -m64/-m32, -O0/-Os): `FLS()`/`LOG2_*` correctness for 32/64/128-bit arguments including compile-time constant contexts; `umulh64()` edge cases and 1M randomized differential tests against the native 128-bit computation; the `up_umulh64()` override dispatch; `div64_const()` correctness for constant and runtime divisors. On qemu-system-x86_64 the measured performance of `div64_const()`/`div_const()` is the same as the division optimized by the compiler (no regression in any scenario). 1-thread ns/op, min of runs: | scene | machine | plain | note | |---|---|---|---| | /1000 | 1.238 | 1.675 | constant: folded multiply-shift | | /1024 | 0.266 | 0.204 | power of two: both a single shift | | /1e9+7 | 0.986 | 1.722 | constant: folded multiply-shift | | /2^40 | 0.203 | 0.217 | power of two: both a single shift | | /(2^32+15) | 1.682 | 1.674 | divisor > 32-bit: both plain division | The sub-cycle differences on the power-of-two rows are code-layout noise. The divisor is expected to be a compile-time constant (see the contract comment on `div64_const()`); for runtime-invariant divisors the `invdiv` API is the right tool and measures 0.379 ns/op versus 1.672 ns/op for plain division (4.4x). Correctness checks pass for all scenes; the 4-thread aggregate scales ~4x. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
