Repository navigation
Fix library bugs found by the reworked tests - #9
Merged
Merged
Conversation
The binary operators took lhs by value and modified it in place. MSVC x86 /O2 doesn't copy over-aligned by value arguments, so the result was written into the caller's register, e.g. Select( m, a * b, a ) read back a * b. Take lhs by const& and copy into a local instead.
The e * -2.12194440e-4f correction multiplied the polynomial instead of being added to it, making FS::Log (and FS::Pow, which uses it) up to ~1.6% wrong, e.g. Log( 1.5 ) gave 0.41211 instead of 0.40547.
_CMP_NEQ_OQ is false when either input is NaN. Use the unordered _CMP_NEQ_UQ, matching SSE cmpneqps, the other platforms and C++ !=.
x86 minps/maxps return their 2nd operand for equal (+-0) or NaN inputs, and WASM min/max and ARM vminq/vmaxq propagate NaN, so feature sets disagreed with each other and with the scalar std::min/std::max. Non relaxed Min/Max now all give b < a ? b : a (and a < b ? b : a): x86 swaps the operands, WASM uses pmin/pmax and ARM selects with vbslq. Relaxed keeps the native ops.
GCC has no implicit conversion between uint32x4_t and float32x4_t or int32x4_t, so casting a mask register didn't compile. Reinterpret every native type through uint32x4_t, picked by element type since with the MSVC ARM64 ABI every NEON vector type is the same __n128.
GCC and Clang fused mul + add into FMA, and -mno-fma isn't enough because AVX512F has its own FMA instructions, so non relaxed results differed between feature sets. clang-cl needs the /clang: form.
Auburn
changed the title
This was referenced Oct 14, 2026
Merged
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
1 participant
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.
Library fixes found by a reworked test harness (the harness itself will follow separately).
FS::Log: thee * -2.12194440e-4fterm used*=instead of+=, making Log (and Pow) up to ~1.6% wrong.!=: used_CMP_NEQ_OQ, which is false when an input is NaN.Min/Max: didn't matchstd::min/std::maxfor +-0 and NaN on x86, WASM and ARM (non relaxed).Castof mask registers: didn't compile with GCC, which has no implicituint32x4_tconversions.-ffp-contract=off. GCC/Clang fused mul + add into FMA (AVX512F has its own FMA even with-mno-fma), so feature sets gave different results.