Re: [PATCH] target/loongarch: Fix incorrect rounding in fnm{add, sub} under certain modes

[WANG Rui]

On Wed, May 7, 2025 at 2:28 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/6/25 10:50, Richard Henderson wrote:
> >> +    *(uint64_t *)&x = 0x4ff0000000000000UL;
> >> +    *(uint64_t *)&y = 0x4ff0000000000000UL;
> >
> >   0x1.0p256
> >
> >> +    *(uint64_t *)&z = 0x2ff0000000000000UL;
> >
> >   0x1.0p-256
> >
> >> +
> >> +    fesetround(FE_DOWNWARD);
> >> +    asm("fnmsub.d %[x], %[x], %[y], %[z]\n\t"
> >> +        :[x]"+f"(x)
> >> +        :[y]"f"(y), [z]"f"(z));
> >> +
> >> +    assert(*(uint64_t *)&x == 0xdfefffffffffffffUL);
> >
> >    -(0x1.0p512 - epsilon)
> >
> > That really should have worked as-is.  I'll have a quick look.
>
> The negate_result is applied early, so we're computing rounding on the 
> -(1p512 - epsilon),
> and since that's negative, round_down produces -1p512.  Whereas what the ISA 
> wants is the
> rounding on (1p512 - epsilon), rounding down to 1.fffffffffffffp511, and only 
> afterward
> negating.
>
> I'm thinking that the current placement of the float_muladd_negate_result 
> test is
> incorrect, since we're not negating the result, we're negating the unrounded 
> intermediate.
>
> There are only 3 targets which use float_muladd_negate_result: loongarch, 
> ppc, and sparc.
> I will use this test case to test those other targets as well.

If the current placement of `float_muladd_negate_result` aligns with
the behavior required by PPC and SPARC, then perhaps we could
introduce a separate flag like `float_muladd_negate_rounded_result`
for LoongArch, applying the negation after rounding instead. What do
you think?

Regards,
-hev

>
>
> r~