Skip to content

riscv: resolve frm for dynamic rounding of fcvt float-to-int - #260

Open
carlosqwqqwq wants to merge 1 commit into
LekKit:stagingfrom
carlosqwqqwq:fix/fcvt-dyn-frm
Open

riscv: resolve frm for dynamic rounding of fcvt float-to-int#260
carlosqwqqwq wants to merge 1 commit into
LekKit:stagingfrom
carlosqwqqwq:fix/fcvt-dyn-frm

Conversation

@carlosqwqqwq

@carlosqwqqwq carlosqwqqwq commented Aug 11, 2026

Copy link
Copy Markdown

Summary

The float-to-integer conversions (fcvt.w.s, fcvt.wu.s, fcvt.l.s, fcvt.lu.s and the .d siblings) pass the raw rm field to the rounding helpers. For rm=dyn the helper falls back to the host-tracked mode, but RMM has no host equivalent and is mapped to RNE, so frm=RMM collapses to round-to-nearest-even instead of round-to-nearest-ties-away. The Zfa fround.s/fround.d paths had the same issue.

Resolve the effective mode from frm when the field is dyn and pass it to the rounding helpers (fcvt float-to-int and fround); static rm values, including RMM, are unchanged. With every caller now resolving DYN, the internal mode > RMM fallback in fpu_round_f32_internal/fpu_round_f64_internal is unreachable and has been removed; the functions now document that the caller always passes a concrete mode (RMM included).

Validation

  • Halfway-input probes (+2.5/-2.5) under static RMM and dyn + frm=RMM for all eight conversion forms match native RISC-V hardware and QEMU.
  • RNE/RDN/RUP controls are unchanged.
  • Clean rebuild passes (make clean + make).

Fixes #255

@LekKit

LekKit commented Aug 11, 2026

Copy link
Copy Markdown
Owner

@SolAstrius please review, thank you

@purplesyringa

Copy link
Copy Markdown
Collaborator

This is not an isolated problem, if we don't handle this more generally it's going to surface again.

The root issue is that fpu_lib assumes DYN denotes the current host mode, while in reality we pretty much always want the current emulated mode. I think every function in fpu_lib needs to have a contract that the passed rounding mode is never DYN, it should never try to default to the host mode, and that logic needs to be moved to the caller that knows what the state of the emulated FPU is.

@LekKit

LekKit commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Technically the host rounding mode is the guest RM_DYN mode. The frm CSR of the guest is directly mapped to the host fegetround() / fesetround() FPU modes. The rounding mode is per-thread, and each vCPU is directly tied to a host thread.
This is supposed behavior by design, as 99% of guest-executed FPU computational instructions (Usual software compiled by Clang/GCC) does not use static RM, and emits fadd/fsub/fmul/fdiv/fsqrt with RM_DYN, and possibly modifies the host rounding mode. This also allows clean JIT interaction in the future, as JIT can directly execute an SSE2/VFP instruction counterpart. Then, fcvt fp->int with RM_RTZ is just a truncating fp->int cast, which is also easily natively representable.

However, this comes with two important exceptions:

  • RM_RMM (Round to Nearest, ties to Max Magnitude) - this rounding mode has no direct counterpart in any other instruction set, other than RISC-V. It must be implemented in software for both static and dynamic RM cases, and disable JIT FPU dispatch while it is active in frm CSR.
  • Hosts without FPU environment support (MIPS, WASM, etc) - those always have RM_RNE rounding mode enabled, and can never set FPU exception flags. They are currently all broken, there was no definitive conclusion as to how to fix this. Ideally, the VM must soft-emulate any combination where static rm is not either RM_RNE or RM_DYN, and frm is not RM_RNE, and must check every instruction to set FPU exception flags in software (thread-local variable, etc).

@purplesyringa

Copy link
Copy Markdown
Collaborator

Technically the host rounding mode is the guest RM_DYN mode. [...] However, this comes with two important exceptions: [...]

I think we're in agreement, but to elaborate: in most instruction handlers, like addition, which don't normally need to bother with the rounding mode, it is indeed a good idea to mostly ignore it and special-case RMM separately. In functions like round, though, which already switch over modes, getting rid of implicit assumptions that are broken under RMM looks reasonable to me, and shouldn't cause any performance issues.

@carlosqwqqwq

Copy link
Copy Markdown
Author

Agreed with the discussion outcome. This PR resolves DYN at the caller (eff_rm = frm CSR when rm==DYN) and passes a concrete mode into fpu_round_*; the helpers never see DYN on the fcvt path, and a guest frm=RMM is honored by the software rounding path. That matches the proposal to move DYN resolution to the caller and keep fpu_lib's contract mode != DYN. No changes needed from the discussion; happy to adjust if you prefer the resolution in a shared helper.

@purplesyringa

Copy link
Copy Markdown
Collaborator

The reason I wrote that comment is that fpu_round_f32_internal still has an if (mode == DYN)-like condition inside it, which as far as I can tell is unreachable after this PR. Could you verify whether that's correct, and if so, drop the condition and update the documentation of that function accordingly? And same for the 64-bit case obviously.

Resolve rm==DYN to the guest frm CSR before rounding in all fcvt
float-to-int and fround (Zfa) paths, so a guest frm=RMM is honored by
the software rounding path instead of silently using the host mode.
fpu_round_{f32,f64}_internal now always receive a concrete mode; their
internal DYN fallback is removed and documented accordingly.
@carlosqwqqwq

Copy link
Copy Markdown
Author

Good catch - verified it. Two things: (1) the fround.s/fround.d (Zfa) paths also fed rm directly into fpu_round_{f32,f64}_to_i64, so the DYN fallback was still reachable; those now use eff_rm too. (2) With every caller resolving DYN, the mode > FPU_LIB_ROUND_MM fallback inside fpu_round_f32_internal / fpu_round_f64_internal is indeed dead: removed it from both and updated the doc comments to state the caller contract (mode is always a concrete mode, RMM included). Rebuilt cleanly and force-pushed.

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.

dyn + frm=RMM is ignored across FCVT.{W,WU,L,LU}.{S,D} conversions

3 participants