Re: [PATCH 2/2] MIPS: Hard-float rounding instructions support

[Xi Ruoyao <xry111@xry111.site>]

On Tue, 2024-01-02 at 17:43 +0800, Junxian Zhu wrote:
> 在 2023/12/26 16:29, Xi Ruoyao 写道:
> > On Tue, 2023-12-26 at 10:37 +0800, Junxian Zhu wrote:
> > > 在 2023/12/25 18:51, Xi Ruoyao 写道:
> > > > On Mon, 2023-12-25 at 18:35 +0800, Junxian Zhu wrote:
> > > > 
> > > > /* snip */
> > > > 
> > > > > +/*
> > > > > + * ceil(x)
> > > > > + * Return x rounded toward -inf to integral value
> > > > > + * Method:
> > > > > + *	Bit twiddling.
> > > > > + */
> > > > > +
> > > > > +#if ((__mips_fpr == 64) && (__mips_hard_float == 1) && ((__mips == 32 && __mips_isa_rev > 1) || __mips == 64))
> > > > > +#include <sys/regdef.h>
> > > > > +#include <sysdep.h>
> > > > > +#include <libm-alias-double.h>
> > > > > +
> > > > > +ENTRY(__ceil)
> > > > > +	.set push
> > > > > +	.set noreorder
> > > > > +	.set noat
> > > > > +# $f0=ret, $f12=double, a0=int64/int32_h, a1=int32_l, a2=sign, a3=exp
> > > > > +#if __mips == 64
> > > > > +	dmfc1   a0, $f12 # assign int64
> > > > > +#else
> > > > > +	mfhc1   a0, $f12 # assign int64
> > > > > +#endif
> > > > > +	cfc1    t0, $f26
> > > > > +	ceil.l.d    $f0, $f12
> > > > No, C23 does not allow this function to raise an INEXACT exception, but
> > > > ceil.l.d will do so.
> > > > 
> > > > Such optimizations should be performed in GCC which can be controlled by
> > > > the programmer with -std=c23 and/or -f[no-]fp-int-builtin-inexact, not
> > > > in Glibc where we cannot know if the programmer wants to deviate from
> > > > C23.
> > > The cfc1 instruction will backup float point exception status before
> > > running ceil.l.d, and the following ctc1 will restore float point
> > > exception status to avoid INEXACT exception raised by ceil.l.d. It's the
> > > same way like what have been done in s_ceil.S for i386.
> > Still incorrect because when the Enable field of FCSR contains INEXACT a
> > SIGFPE will be immediately delivered and there is no way to recover.  A
> > demonstration:
> > 
> > #define _GNU_SOURCE
> > #include <stdio.h>
> > #include <fenv.h>
> > 
> > int main()
> > {
> >    printf("%d\n", feenableexcept(FE_INEXACT));
> > 
> >    double data = 114.514;
> >    long control;
> >    asm("cfc1\t%1,$f26\n\t"
> >        "ceil.l.d\t%0,%0\n\t"
> >        "cvt.d.l\t%0,%0\n\t"
> >        "ctc1\t%1,$f26": "+f"(data), "=r"(control));
> >    printf("%.15f\n", data);
> >    return 0;
> > }
> > 
> > On i386 the fnstenv instruction also masks out all the FP exceptions so
> > this is not a problem.  See commit 26b0bf96000a.
> 
> I can use "ctc1 $0, $28" to disable all float point exception to ensure 
> no FP exceptions occur at here. But it will introduce additional 
> consumption.

And then it will likely be even slower than the generic implementation
like Adhemerval already tested on the cfarm machine.  Frankly I'm even
unsure if your (incorrect) implementation is really faster than the
generic implementation: if the uarch just handles all ctc1 instructions
equally (i.e. always stalling the FP unit or even the entire CPU for a
dozen of cycles) it would be already slower.

Have you benchmarked this on real hardware?  Note that benchmarking on
things like QEMU can be completely misleading.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University