riscv_fpu: fsqrt(-0.0), NaN-boxing, and FMA underflow-flag fixes#238
Closed
SolAstrius wants to merge 8 commits into
Closed
riscv_fpu: fsqrt(-0.0), NaN-boxing, and FMA underflow-flag fixes#238SolAstrius wants to merge 8 commits into
SolAstrius wants to merge 8 commits into
Conversation
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
… after rounding) Signed-off-by: Sol Astrius Phoenix <sol@astrius.ink>
Contributor
Author
|
Closing in favor of a single consolidated patch, per the maintainer's request to submit this as one patch rather than a stacked-PR series. The combined FPU work (RMM rounding, static rounding modes, NaN canonicalization, FMA rounding/ties/underflow, fmul ties, fcvt-to-int flags) now passes the riscv-arch-test F/D/I/M suites 260/260. Rebased onto current staging (which already includes #219). |
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.
Summary
Closes three more FPU conformance gaps surfaced by the arch-test diagnostic. Draft — stacked on #237 → #236 → #233 (shows their commits until they land; the new work is the top three:
fsqrt(-0.0), NaN-boxing, FMA spurious-UF). Posted as a checkpoint to document findings; the remaining tail (below) is in progress.Combined with the stack, arch-test (ACT4 / Spike) goes 101 → 216 / 260 (I 51/51, M 13/13, F 13→58, D 24→94). MPFR RMM harness still 3006/3006, no value regressions.
The three fixes
fpu_lib: sqrt(-0.0)returns-0.0—fpu_is_negative32/64(-0.0)is true (−0.0 is the most-negative sign-magnitude int), sofsqrt(-0.0)wrongly raisedNVand returned canonical NaN. Exclude ±0 from the negative branch; spec wantssqrt(-0.0) = -0.0, no exception.Treat mal-boxed narrow operands as the canonical NaN — value-consuming
.sops read operands viariscv_view_s(raw low 32 bits), ignoring NaN-boxing. RISC-V requires an improperly-boxedf32(upper bits ≠ all-ones) to be read as the canonical NaN. Switched all value ops to the boxing-awareriscv_read_s(identical for properly-boxed inputs; onlyfmv.x.wkeeps the raw read). Clears most of the D-suite narrow-op tests (+23 D).Drop the spurious FMA underflow flag — RISC-V detects tininess after rounding; some hosts (aarch64) detect it before, so an FMA result that rounds up to the smallest normal spuriously sets
UF.UFis valid only for a subnormal result, so it's cleared when the result is exactly the smallest normal. (See the perf note — this one has a tradeoff.)Performance (fp-bench microbench, min ticks, aarch64; lower = faster)
view_s→read_sNaN-box check is effectively free.Remaining gaps (in progress / out of scope)
fpu_mul_error/Dekker) and the plain FMA don't yield an exact tie decision for subnormal results; need an FMA-based exact residual / error-free FMA. These are gated behind the (rare) RMM path, so they carry no common-path perf cost. Not yet implemented.Harness + per-instruction diagnosis: https://github.com/SolAstrius/rvvm-conformance