Repository navigation
fix(fpu): round fused multiply-add only once - #11
Merged
Merged
Conversation
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.
Failure
fmadd.dcurrently returns zero forThe exact answer is
-2^-104(0xb970000000000000). The multiplier rounds the product to1before the addend is applied, so cancellation destroys a representable result.RISC-V's fused operations compute the product and sum before rounding once. The intermediate rounding also changes range behavior:
maxFinite * 2 - maxFinitebecomes infinity rather thanmaxFinite, andminSubnormal * 0.5 + minSubnormalreturns one rather than two minimum subnormals under RNE. These errors affect numerical results and would make exception reporting based on the intermediate multiply incorrect.Fix
Keep the existing multiplier, retain its full product, and align/add the third operand before reducing the result to the existing round/pack path. Cancellation therefore preserves the product's low bits, and intermediate product overflow/underflow is not mistaken for overflow/underflow of the fused result. Three-operand special cases and all four product/addend sign combinations are handled directly.
The ordinary add, multiply, divide and conversion paths remain. The FMA reference tests now use exact integer significands/exponents rather than Dart's separately rounded
a * b + cexpression.Boundaries
This fixes single rounding under the unit's existing RNE behavior. Architectural rounding-mode selection and exception-flag reporting remain separate follow-up integration; this PR does not claim to implement them. It adds full-product alignment/addition state and changes FMA latency, but introduces no timing pipeline or board changes. Area/timing, synthesis and hardware have not been validated.
Validation
Against
f403d31, the same 55 named tests run in fresh processes give 34 pass / 21 fail → 53 pass / 2 fail, with no lost passes or missing cases.fma/fmafon 21,664 cases, with NaNs canonicalized.The two remaining failures are identical baseline F32 move/boxing failures: the load/store round-trip expects
123but gets-4294967173; the sign/min/max/class/move case expects1065353216but gets-3229614080. Neither is changed or counted as an FMA regression. This is targeted Dart simulation, not a full-suite or external-RTL validation claim.