On 8/14/25 8:23 AM, Vineet Gupta wrote:
__builtin_round() fails to save/restore FP exception flags around the FP
compare insn which can potentially clobber the same.

Worth noting that the fflags restore bracketing is slightly different
than the glibc implementation. gcc implementation w/ this patch
generates following, where fsflags is called even if fcvt* were not
executed because the prior flt can also clobber the flags. glibc
implementation due to early NaN check (before the flt.s) only needs
inside of the branch.

| convert_float_to_float_round
| ...
|   frflags     a5
|   fabs.s      fa5,fa0
|   flt.s       a4,fa5,fa4    <--- can clobber fflags
|   beq a4,zero,.L3
|     fcvt.w.s a4,fa0,rmm     <--- also
|     fcvt.s.w  fa5,a4
|     fsgnj.s   fa0,fa5,fa0
| .L3:
|    fsflags    a5            <-- both code paths

        PR target/121534

gcc/ChangeLog:

        * config/riscv/riscv.md (round_pattern): save/restore fflags.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/round_64.c: Scan for frflags and fsflags.
So I'm guessing the issue at hand is in that rounding path that does not use Zfa we've introduced a flt instruction where we didn't have one before (previously we likey called into glibc). And so now we have to manage the accrued exception state? Essentially saving/restoring state around the round() case?

My recollection is that sequence was dramatically better than what we had before, so even with a bit mucking around with FP CSRs it's likely still profitable.

It failed CI, presumably a scan test:

https://github.com/ewlu/gcc-precommit-ci/issues/3774#issuecomment-3188777455

Assuming the scan test just needs the count updated, this is OK for the trunk.

Thanks,
Jeff

Reply via email to