CCMath 0.3.0 Release Candidate#149
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| { | ||
| return static_cast<T>(1) / x; | ||
| } | ||
| { return static_cast<T>(1) / x; } |
There was a problem hiding this comment.
Casting of literals is inconsistent between files, sometimes T(123) and sometimes static_cast<T>(123)
| { | ||
| const T length = src_end - src_start; | ||
|
|
||
| if (length == T(0)) { return dst_start; } |
There was a problem hiding this comment.
Does it make sense to compare floating-point values with ==? Perhaps approximately() can be used instead. This might also apply to other files.
| template <typename T> | ||
| inline constexpr bool has_constexpr_abs = | ||
| #ifdef CCMATH_HAS_CONSTEXPR_BUILTIN_ABS | ||
| std::is_same_v<T, float> || std::is_same_v<T, double> || std::is_same_v<T, long double> || std::is_same_v<T, long long> || ( |
There was a problem hiding this comment.
Did you mean to std::remove_cv_t<T>? Checking std::is_same_v<T, long long> seems redundant because of the std::is_integral_v<T> check. Checking whether T is signed or unsigned seems redundant too, and is this intended to allow bool?
Also, it might be beneficial to make a helper:
template <typename First, typename... Rest>
inline constexpr bool is_same_any = (... || std::is_same_v<First, Rest>);
// ...
template <typename T>
inline constexpr bool has_constexpr_abs = is_same_any<std::remove_cv_t<T>, float, double, long double> // ...| @@ -65,9 +65,7 @@ namespace ccm | |||
|
|
|||
| template <typename T, typename U, typename TC = std::common_type_t<T, U>> | |||
There was a problem hiding this comment.
Is TC intended to be customizable by the user?
| template <typename... Fs> | ||
| struct flags_align_request<simd_flags<Fs...>> | ||
| : std::integral_constant<std::size_t, | ||
| ((std::size_t{ 0 } + ... + overalign_value<Fs>::value) != 0) |
There was a problem hiding this comment.
More inconsistent casting of literals, now with curly braces
There was a problem hiding this comment.
I could enforce a convention with an AST matcher but I'm not sure if it's worth the effort or not. Thoughts?
CCMath 0.3.0 Release Candidate
This is the review PR for v0.3.0. The goal is to catch any remaining blockers before the
release is cut.
It puts the entire v0.2.0 to v0.3.0 change set in one diff. It is review-only and will not be
merged. The base branch
review-base/v0.2.0is a frozen snapshot of thev0.2.0tag, so thediff is exactly what landed since the last release. The content itself already lives on
mainand on
release/0.3.x.How this review works: comments here are the review record. Fixes land as normal PRs into
main, after whichrelease/0.3.xis fast-forwarded. Record your approval as a review here.When review wraps up, the PR is closed and
review-base/v0.2.0is deleted. Treat it as a diffviewer and discussion thread.
The user-facing summary is in the release notes. What follows is for reviewers: what changed,
where the risk is, what is validated, and what would block the release.
What counts as blocking
Reasons to hold the release. Mark these clearly so they stand out from nits:
mode, on a supported path.
powlcall routed to the wrong kernel).extorfmanipfunctions (signature, NaN andinfinity behavior, constexpr-ness).
Most other items can be handled as nits or follow-ups unless they change the release risk.
Items under Known limitations are out of scope by design, not blockers, unless you can show
one breaks a supported path.
Where to focus review
These are the surfaces where a mistake means wrong math, so they earn the closest reading.
Validation has been run on most of them (see Validation status), but that lowers the
uncertainty rather than removing the need for attention.
Trigonometry (
doublesin,cos,tan)sin,cos, andtankernels.quadrant selection, sign-of-zero handling, and the inverse-trig edge cases (
asin,acos,atan,atan2). The earlier failure mode was large arguments falling outside thereduction, so large-input behavior is the thing to primarily probe.
Power (
doublepow,long doublepowl)powpath. The headline bug class fixed thisrelease was hi and lo halves stored swapped, which is harmless under round-to-nearest but
wrong in directed modes, so check each DD constant against its intended value and ordering.
powlkernel:powl_ld80_kernel.hpp
and its generated
powl_ld80_tables.hpp.
It compiles only where
long doubleis the 80-bit x87 extended format, so thedoubleandIEEE binary128
long doubleplatforms do not exercise it. This is preview quality (seeKnown limitations).
powpaths:pow_simd_impl.hppandpowf_simd_impl.hppin the same directory. Theserun only under runtime SIMD (not deterministic mode), so the check is that they agree with
the scalar path on the same inputs.
Configuration and dispatch
powl_policy.hpp decide which
long doublepath is taken. A wrong detection sends every
powlcall down the wrong kernel, so theformat-detection logic is worth careful eyes.
CCMATH_ENABLE_DETERMINISTIC): the claim is bit-identicalfloatanddoubleresults across supported hardware. Worth confirming it forces the generic kernels, disablesruntime SIMD, does not leak into a libm or compiler builtin on any path, and that constexpr
and runtime agree.
long doubleis not part of the deterministic claim.New public API
ccm::exthelpers under ext/ and the standardnextupand
nextdownunder fmanip/. Check signatures, NaN andinfinity behavior, constexpr-ness, and that they build clean under
-Wconversion. Theexthelpers are opt-in behind
CCMATH_ENABLE_EXTENSIONS, so also check the namespace placementand that they stay out of a default build.
Low-level support
cast.hpp,except_value_utils.hpp,fma.hpp. The changes here are small, but used very widely so mistake can easily propagate.Lower-risk areas (skim)
ccm::ppSIMD layer under runtime/pp/: astandalone C++17 port of the C++26
std::simdinterface. Not yet wired into dispatch, so itcannot change results this release. Review it on its own merits if you have the appetite,
otherwise it can wait.
question here is coverage gaps against the kernels above.
benchmarks. Build and dev infrastructure, not part of library runtime behavior.
meson.build,premake5.lua, the CMake options, and the vendored-consumerintegration test.
per-PR clang-format gate.
POW_PROOF.tex(the full binary64 proof is still open) and theSollya and Gappa scripts under
docs/approximating_functions.Validation status
What has been validated, and how. This lowers the uncertainty on these areas, it does not put
them out of bounds for review:
pow: CORE-MATH all-mode validation campaigns over the covered inputs, all four roundingmodes. The campaign records live under
tests/rigorous/(seeoracle_logs/for theper-campaign summaries).
sin,cos,tan: full-range validation campaigns, large arguments included. This isrepresentative coverage across the range, not an exhaustive sweep of every binary64 value.
fma: all-mode exact software fallback, with native runtime coverage validated on AArch64.nearest, power, and trig families.
-Werrorwith aggressive warnings (-Wconversionand friends) across the CImatrix on Linux, macOS, and Windows.
Open validation work (not expected to be finished in this RC):
powproof. The writeup in the tree is partial.powfreduced-domain validation.powlkernel.fmavalidation.Known limitations and intended scope
These are disclosed in the release notes and are intentionally out of scope for v0.3.0. Call
one out if you think it should block the release, otherwise treat it as known debt:
powlkernel is preview quality. Its generic fuzz lane is intentionally guardedoff in power_fuzz.cpp until the kernel is finalized.
Building and validating locally
It is a header-only library, so consuming it is just an include. To exercise the validation
that matters for review:
cmake -S . -B build -DCCMATH_BUILD_RIGOROUS_TESTS=ON \ -DCCMATH_ENABLE_WARNINGS_AS_ERRORS=ON -DCCMATH_ENABLE_AGGRESSIVE_WARNINGS=ON cmake --build build --config Release ctest --test-dir build --output-on-failure -C ReleaseOn multi-config generators like MSVC the
--configand-C Releaseflags matter, and theyare harmless elsewhere. Add
-DCCMATH_ENABLE_MPFR_TESTS=ONand-DCCMATH_ENABLE_COREMATH_TESTS=ONfor the oracle-backed campaigns (these need MPFR andCORE-MATH available, and the rigorous runs can take a while). Fuzzers build under Clang with
-DCCMATH_BUILD_FUZZING=ON. Runtools/ensure_format.shfor the format gate. The READMEvalidation section and
tools/asmlab/README.mdcover the deeper tooling.Where review is most valuable
Highest value first: numeric correctness and edge cases in the kernels above, then the public
API shape of the new
extandfmanipfunctions, then test coverage gaps against thosekernels. Style and tooling nits are welcome but lowest priority for an RC this size.