Simulation of MIPS recip and rsqrt instructions

Simulation of MIPS recip and rsqrt instructions

[Richard Sandiford]

The mips simulator uses sim_fpu_inv() as a reciprocal function
but all it really does is negate the exponent.  So we end up
calculating 1 / (x*2^y) as x*2^-y.

Test case:

    #include <stdio.h>

    int
    main ()
    {
      double d;
      float f;

      asm ("recip.s %0,%1" : "=f" (f) : "f" (20.0f));
      printf ("%g\n", f);

      asm ("recip.d %0,%1" : "=f" (d) : "f" (10.0));
      printf ("%g\n", d);

      asm ("rsqrt.s %0,%1" : "=f" (f) : "f" (0.01f));
      printf ("%g\n", f);

      asm ("rsqrt.d %0,%1" : "=f" (d) : "f" (25.0));
      printf ("%g\n", d);

      return 0;
    }

The simulator says:

    0.078125
    0.15625
    25.6
    0.3125

while a vr5500 says:

    0.05
    0.1
    10
    0.2

Unfortunately, sim_fpu_inv isn't commented and cp1.c seems to be
its only active user.  Which should change?

FWIW, the patch below fixes the test case.  Please install if OK.

Richard


	* cp1.c (inner_sqrt): Use sim_fpu_div instead of sim_fpu_inv.
	(fpu_inv): New function.
	(fp_recip): Use it.

Index: sim/mips/cp1.c
===================================================================
RCS file: /cvs/src/src/sim/mips/cp1.c,v
retrieving revision 1.19
diff -u -d -p -F^[(a-zA-Z0-9_^#] -r1.19 cp1.c
--- sim/mips/cp1.c	29 Jul 2002 23:17:10 -0000	1.19
+++ sim/mips/cp1.c	26 Nov 2002 19:13:24 -0000
@@ -900,7 +900,7 @@ inner_rsqrt(unsigned64 op1,
 	status |= sim_fpu_sqrt (&ans, &wop1);
 	status |= sim_fpu_round_32 (&ans, status, round);
 	wop1 = ans;
-	op_status = sim_fpu_inv (&ans, &wop1);
+	op_status = sim_fpu_div (&ans, &sim_fpu_one, &wop1);
 	op_status |= sim_fpu_round_32 (&ans, round, denorm);
 	sim_fpu_to32 (&res, &ans);
 	temp = res;
@@ -914,7 +914,7 @@ inner_rsqrt(unsigned64 op1,
 	status |= sim_fpu_sqrt (&ans, &wop1);
 	status |= sim_fpu_round_64 (&ans, round, denorm);
 	wop1 = ans;
-	op_status = sim_fpu_inv (&ans, &wop1);
+	op_status = sim_fpu_div (&ans, &sim_fpu_one, &wop1);
 	op_status |= sim_fpu_round_64 (&ans, round, denorm);
 	sim_fpu_to64 (&res, &ans);
 	temp = res;
@@ -1027,13 +1027,19 @@ fp_div(sim_cpu *cpu,
   return fp_binary(cpu, cia, &sim_fpu_div, op1, op2, fmt);
 }
 
+static int
+fpu_inv(sim_fpu *f, const sim_fpu *l)
+{
+  return sim_fpu_div (f, &sim_fpu_one, l);
+}
+
 unsigned64
 fp_recip(sim_cpu *cpu,
          address_word cia,
          unsigned64 op,
          FP_formats fmt)
 {
-  return fp_unary(cpu, cia, &sim_fpu_inv, op, fmt);
+  return fp_unary(cpu, cia, &fpu_inv, op, fmt);
 }
 
 unsigned64

Re: Simulation of MIPS recip and rsqrt instructions

[Andrew Cagney]

> The mips simulator uses sim_fpu_inv() as a reciprocal function
> but all it really does is negate the exponent.  So we end up
> calculating 1 / (x*2^y) as x*2^-y.
> 
> Test case:
> 
>     #include <stdio.h>
> 
>     int
>     main ()
>     {
>       double d;
>       float f;
> 
>       asm ("recip.s %0,%1" : "=f" (f) : "f" (20.0f));
>       printf ("%g\n", f);
> 
>       asm ("recip.d %0,%1" : "=f" (d) : "f" (10.0));
>       printf ("%g\n", d);
> 
>       asm ("rsqrt.s %0,%1" : "=f" (f) : "f" (0.01f));
>       printf ("%g\n", f);
> 
>       asm ("rsqrt.d %0,%1" : "=f" (d) : "f" (25.0));
>       printf ("%g\n", d);
> 
>       return 0;
>     }

Can this test case be written in strict assembler so that it can be 
added to the simulator testsuite?

> The simulator says:
> 
>     0.078125
>     0.15625
>     25.6
>     0.3125
> 
> while a vr5500 says:
> 
>     0.05
>     0.1
>     10
>     0.2
> 
> Unfortunately, sim_fpu_inv isn't commented and cp1.c seems to be
> its only active user.  Which should change?
> 
> FWIW, the patch below fixes the test case.  Please install if OK.

Hmm, I guess:

	1 / (x * 2^y) == (1/x) * (1/(2^y)) == (1/x) * (2^(-y))

and
	1/x != x

so I think the inv() function should be fixed - that suggests it is 
pretty broken.

Does GLIBC's softfloat code contain an efficient INV implementation that 
could be used?

Andrew

Re: Simulation of MIPS recip and rsqrt instructions

[Richard Sandiford]

Andrew Cagney <ac131313@redhat.com> writes:
> Can this test case be written in strict assembler so that it can be
> added to the simulator testsuite?

Is there a testsuite for mips?

> > Unfortunately, sim_fpu_inv isn't commented and cp1.c seems to be
> > its only active user.  Which should change?
> 
> Hmm, I guess:
> 
> 	1 / (x * 2^y) == (1/x) * (1/(2^y)) == (1/x) * (2^(-y))
> 
> and
> 	1/x != x
> 
> so I think the inv() function should be fixed - that suggests it is
> pretty broken.

OK, so sim_fpu_inv() _is_ supposed to be a general reciprocal function?
Not just for powers of two, etc.?  It seemed so deliberate, it was
difficult to tell.

Like you say, there's probably some code that could be nicked
from elsewhere, but reusing sim_fpu_div should work too, even
if it isn't as efficient.  The patch below also fixes the
test case.

Richard


Index: sim/common/sim-fpu.c
===================================================================
RCS file: /cvs/src/src/sim/common/sim-fpu.c,v
retrieving revision 1.4
diff -c -d -p -F^[(a-zA-Z0-9_^#] -r1.4 sim-fpu.c
*** sim/common/sim-fpu.c	12 Jun 2002 00:46:11 -0000	1.4
--- sim/common/sim-fpu.c	26 Nov 2002 20:33:46 -0000
*************** INLINE_SIM_FPU (int)
*** 1754,1786 ****
  sim_fpu_inv (sim_fpu *f,
  	     const sim_fpu *r)
  {
!   if (sim_fpu_is_snan (r))
!     {
!       *f = *r;
!       f->class = sim_fpu_class_qnan;
!       return sim_fpu_status_invalid_snan;
!     }
!   if (sim_fpu_is_qnan (r))
!     {
!       *f = *r;
!       f->class = sim_fpu_class_qnan;
!       return 0;
!     }
!   if (sim_fpu_is_infinity (r))
!     {
!       *f = sim_fpu_zero;
!       f->sign = r->sign;
!       return 0;
!     }
!   if (sim_fpu_is_zero (r))
!     {
!       f->class = sim_fpu_class_infinity;
!       f->sign = r->sign;
!       return sim_fpu_status_invalid_div0;
!     }
!   *f = *r;
!   f->normal_exp = - r->normal_exp;
!   return 0;
  }
  
  
--- 1754,1760 ----
  sim_fpu_inv (sim_fpu *f,
  	     const sim_fpu *r)
  {
!   return sim_fpu_div (f, &sim_fpu_one, r);
  }
  
  

Re: Simulation of MIPS recip and rsqrt instructions

[cgd]

At Tue, 26 Nov 2002 19:42:02 +0000 (UTC), "Richard Sandiford" wrote:
> The mips simulator uses sim_fpu_inv() as a reciprocal function
> but all it really does is negate the exponent.  So we end up
> calculating 1 / (x*2^y) as x*2^-y.

This would appear to be my bad:

In adding those calls to sim_fpu_inv() I did not notice the changes to
sim_fpu_inv() in our source which replace its entire body with:

#if 0
	... old body ...
#else
  const sim_fpu sim_fpu_one = {
    sim_fpu_class_number, 0, IMPLICIT_1, 0
  };  
  return sim_fpu_div (f, &sim_fpu_one, r, sim_fpu_round_default);
#endif

8-)


> Unfortunately, sim_fpu_inv isn't commented and cp1.c seems to be
> its only active user.  Which should change?

For small values of only.  8-)

common/cgen-accfp.c seems to use it as well for "invsf" and "invdf".

I don't know whether the code in cgen-accfp.c is correct given the
current definition of sim_fpu_inv().  I'd guess not.

(The 'cgen/doc' information about "inv" that i could find ... makes me
think the documentation there isn't complete.  Certainly, it doesn't
seem to match the sim-fpu.h code either...)


My personal preference would be to fix sim_fpu_inv().  it seems more
sensible to me.

Anybody got a clue as to what the cgen-accfp.c 'invsf' and 'invdf' fns
are supposed to do?



chris

Re: Simulation of MIPS recip and rsqrt instructions

[Richard Sandiford]

cgd@broadcom.com writes:
> In adding those calls to sim_fpu_inv() I did not notice the changes to
> sim_fpu_inv() in our source which replace its entire body with:
> 
> #if 0
> 	... old body ...
> #else
>   const sim_fpu sim_fpu_one = {
>     sim_fpu_class_number, 0, IMPLICIT_1, 0
>   };  
>   return sim_fpu_div (f, &sim_fpu_one, r, sim_fpu_round_default);
> #endif
> 
> 8-)

Ah. ;)  Out of interest, how come you're using these local constants?
I noticed the same thing in cp1.c.  Is it just supposed to lead to
better optimisation than the global sim_fpu_one?

> > Unfortunately, sim_fpu_inv isn't commented and cp1.c seems to be
> > its only active user.  Which should change?
> 
> For small values of only.  8-)
> 
> common/cgen-accfp.c seems to use it as well for "invsf" and "invdf".

Yeah, but nothing seems to use invsf and invdf.  AFAICT, the mips
port really is the only active user.

> Anybody got a clue as to what the cgen-accfp.c 'invsf' and 'invdf' fns
> are supposed to do?

Not me.  I looked at the cgen docs, but it didn't mention it
explicitly.  But like you & Andrew say, it's probably better
to patch sim_fpu_inv().  Please install my second patch if it's
OK.  I realise I missed the change log first time, so here it
is again.

Richard

	* sim-fpu.c (sim_fpu_inv): Use sim_fpu_div.

Index: sim/common/sim-fpu.c
===================================================================
RCS file: /cvs/src/src/sim/common/sim-fpu.c,v
retrieving revision 1.4
diff -c -d -p -F^[(a-zA-Z0-9_^#] -r1.4 sim-fpu.c
*** sim/common/sim-fpu.c	12 Jun 2002 00:46:11 -0000	1.4
--- sim/common/sim-fpu.c	26 Nov 2002 20:33:46 -0000
*************** INLINE_SIM_FPU (int)
*** 1754,1786 ****
  sim_fpu_inv (sim_fpu *f,
  	     const sim_fpu *r)
  {
!   if (sim_fpu_is_snan (r))
!     {
!       *f = *r;
!       f->class = sim_fpu_class_qnan;
!       return sim_fpu_status_invalid_snan;
!     }
!   if (sim_fpu_is_qnan (r))
!     {
!       *f = *r;
!       f->class = sim_fpu_class_qnan;
!       return 0;
!     }
!   if (sim_fpu_is_infinity (r))
!     {
!       *f = sim_fpu_zero;
!       f->sign = r->sign;
!       return 0;
!     }
!   if (sim_fpu_is_zero (r))
!     {
!       f->class = sim_fpu_class_infinity;
!       f->sign = r->sign;
!       return sim_fpu_status_invalid_div0;
!     }
!   *f = *r;
!   f->normal_exp = - r->normal_exp;
!   return 0;
  }
  
  
--- 1754,1760 ----
  sim_fpu_inv (sim_fpu *f,
  	     const sim_fpu *r)
  {
!   return sim_fpu_div (f, &sim_fpu_one, r);
  }
  
  

Re: Simulation of MIPS recip and rsqrt instructions

[cgd]

At Wed, 27 Nov 2002 08:12:20 +0000 (UTC), "Richard Sandiford" wrote:
> 	* sim-fpu.c (sim_fpu_inv): Use sim_fpu_div.

I probably overstepped my authority, but I just installed this.

thanks.


chris