Skip to content

riscv_fpu: fsqrt(-0.0), NaN-boxing, and FMA underflow-flag fixes#238

Closed
SolAstrius wants to merge 8 commits into
LekKit:stagingfrom
pufit:fix/fpu-gaps
Closed

riscv_fpu: fsqrt(-0.0), NaN-boxing, and FMA underflow-flag fixes#238
SolAstrius wants to merge 8 commits into
LekKit:stagingfrom
pufit:fix/fpu-gaps

Conversation

@SolAstrius

@SolAstrius SolAstrius commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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

  1. fpu_lib: sqrt(-0.0) returns -0.0fpu_is_negative32/64(-0.0) is true (−0.0 is the most-negative sign-magnitude int), so fsqrt(-0.0) wrongly raised NV and returned canonical NaN. Exclude ±0 from the negative branch; spec wants sqrt(-0.0) = -0.0, no exception.

  2. Treat mal-boxed narrow operands as the canonical NaN — value-consuming .s ops read operands via riscv_view_s (raw low 32 bits), ignoring NaN-boxing. RISC-V requires an improperly-boxed f32 (upper bits ≠ all-ones) to be read as the canonical NaN. Switched all value ops to the boxing-aware riscv_read_s (identical for properly-boxed inputs; only fmv.x.w keeps the raw read). Clears most of the D-suite narrow-op tests (+23 D).

  3. 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. UF is 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)

staging #237 stack this branch
addmul (f64) 20.8M 11.7M 11.9M
div (f64) 13.7M 6.56M 6.5M
sqrt (f64) 31.3M 14.4M 13.6M
addmul.s (f32) 19.8M 12.1M 12.2M
fma (f64) 6.93M 7.50M 8.50M
  • f64 arith and f32 arith are at parity with the stack — the view_sread_s NaN-box check is effectively free.
  • The FMA UF fixup costs ~13% on this FMA-only microbench (it inspects every FMA result, an FP→GP move). Real-world impact is expected far smaller (FMA is a fraction of FP, which is a fraction of mostly-JIT'd-integer workloads — cf. riscv_fpu: don't size-optimize the FP op dispatch #234's 2× micro → ~3% real).

Remaining gaps (in progress / out of scope)

  • FMA RMM exact ties and fmul subnormal-result RMM ties — the error-free transforms (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.
  • fcvt-to-int inexact flags and the FMA invalid flag — owned by @whensun's Fix RISC-V FPU conformance issues #219; intentionally not duplicated here.

Harness + per-instruction diagnosis: https://github.com/SolAstrius/rvvm-conformance

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>
@SolAstrius

Copy link
Copy Markdown
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).

@SolAstrius SolAstrius closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant