Patchwork [v5,1/6] sim: cgen: add remainder functions (needed for OR1K lf.rem.[sd])

login
register
mail settings
Submitter Stafford Horne
Date Oct. 5, 2017, 1:49 p.m.
Message ID <20171005134912.26799-2-shorne@gmail.com>
Download mbox | patch
Permalink /patch/23343/
State New
Headers show

Comments

Stafford Horne - Oct. 5, 2017, 1:49 p.m.
From: Peter Gavin <pgavin@gmail.com>

* sim/common/ChangeLog:

2016-05-21  Peter Gavin  <pgavin@gmail.com>
	    Stafford Horne <shorne@gmail.com>

	* cgen-accfp.c (remsf, remdf): New function.
	(cgen_init_accurate_fpu): Add remsf and remdf.
	* cgen-fpu.h (cgen_fp_ops): Add remsf, remdf, remxf and remtf.
	* sim-fpu.c (sim_fpu_rem): New function.
	* sim-fpu.h (sim_fpu_status_invalid_irx): New enum.
	(sim_fpu_rem): New function.
---
 sim/common/cgen-accfp.c | 40 ++++++++++++++++++++++
 sim/common/cgen-fpu.h   |  4 +++
 sim/common/sim-fpu.c    | 90 +++++++++++++++++++++++++++++++++++++++++++++++--
 sim/common/sim-fpu.h    | 13 ++++---
 4 files changed, 140 insertions(+), 7 deletions(-)
Simon Marchi - Oct. 7, 2017, 3:52 p.m.
As far as I can tell, this patch looks good, but I'm more or less clueless about floating point stuff...

I just pointed out nits:

On 2017-10-05 09:49 AM, Stafford Horne wrote:
> From: Peter Gavin <pgavin@gmail.com>
>
> * sim/common/ChangeLog:
>
> 2016-05-21  Peter Gavin  <pgavin@gmail.com>
> 	    Stafford Horne <shorne@gmail.com>
>
> 	* cgen-accfp.c (remsf, remdf): New function.
> 	(cgen_init_accurate_fpu): Add remsf and remdf.
> 	* cgen-fpu.h (cgen_fp_ops): Add remsf, remdf, remxf and remtf.
> 	* sim-fpu.c (sim_fpu_rem): New function.

Mention the change to sim_fpu_print_status.

sim-fpu.c contains other changes (comments added, lines removed).  If you think the changes belong with this patch, mention them here, otherwise submit them as a separate patch.
> @@ -1551,6 +1551,89 @@ sim_fpu_div (sim_fpu *f,
>
>
>  INLINE_SIM_FPU (int)
> +sim_fpu_rem (sim_fpu *f,
> +	     const sim_fpu *l,
> +	     const sim_fpu *r)
> +{
> +  if (sim_fpu_is_snan (l))
> +    {
> +      *f = *l;
> +      f->class = sim_fpu_class_qnan;
> +      return sim_fpu_status_invalid_snan;
> +    }
> +  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 (l))
> +    {
> +      *f = *l;
> +      f->class = sim_fpu_class_qnan;
> +      return 0;
> +    }
> +  if (sim_fpu_is_qnan (r))
> +    {
> +      *f = *r;
> +      f->class = sim_fpu_class_qnan;
> +      return 0;
> +    }
> +  if (sim_fpu_is_infinity (l))
> +    {
> +      *f = sim_fpu_qnan;
> +      return sim_fpu_status_invalid_irx;
> +    }
> +  if (sim_fpu_is_zero (r))
> +    {
> +      *f = sim_fpu_qnan;
> +      return sim_fpu_status_invalid_div0;
> +    }
> +  if (sim_fpu_is_zero (l))
> +    {
> +      *f = *l;
> +      return 0;
> +    }
> +  if (sim_fpu_is_infinity (r))
> +    {
> +      *f = *l;
> +      return 0;
> +    }
> +  {
> +    sim_fpu n, tmp;
> +
> +    /* Remainder is calculated as l-n*r, where n is l/r rounded to the
> +       nearest integer.  The variable n is rounded half even.  */
> +
> +    sim_fpu_div (&n, l, r);
> +    sim_fpu_round_64 (&n, 0, 0);
> +
> +    if (n.normal_exp < -1) /* If n looks like zero just return l.  */
> +      {
> +	*f = *l;
> +	return 0;
> +      }
> +    else if (n.class == sim_fpu_class_number
> +	&& n.normal_exp <= (NR_FRAC_GUARD)) /* If not too large round.  */

This line should be aligned with the opening parenthesis (well, one char to the right).

> +      do_normal_round (&n, (NR_FRAC_GUARD) - n.normal_exp, sim_fpu_round_near);
> +
> +    /* Mark 0's as zero so multiply can detect zero.  */
> +    if (n.fraction == 0)
> +      n.class = sim_fpu_class_zero;
> +
> +    /* Calculate n*r.  */
> +    sim_fpu_mul (&tmp, &n, r);
> +    sim_fpu_round_64 (&tmp, 0, 0);
> +
> +    /* Finally calculate l-n*r.  */
> +    sim_fpu_sub (f, l, &tmp);
> +
> +    return 0;
> +  }
> +}

Thanks,

Simon
Stafford Horne - Oct. 8, 2017, 12:23 p.m.
On Sat, Oct 07, 2017 at 11:52:30AM -0400, Simon Marchi wrote:
> As far as I can tell, this patch looks good, but I'm more or less clueless about floating point stuff...
> 
> I just pointed out nits:
> 
> On 2017-10-05 09:49 AM, Stafford Horne wrote:
> > From: Peter Gavin <pgavin@gmail.com>
> >
> > * sim/common/ChangeLog:
> >
> > 2016-05-21  Peter Gavin  <pgavin@gmail.com>
> > 	    Stafford Horne <shorne@gmail.com>
> >
> > 	* cgen-accfp.c (remsf, remdf): New function.
> > 	(cgen_init_accurate_fpu): Add remsf and remdf.
> > 	* cgen-fpu.h (cgen_fp_ops): Add remsf, remdf, remxf and remtf.
> > 	* sim-fpu.c (sim_fpu_rem): New function.
> 
> Mention the change to sim_fpu_print_status.

Right, Thank you.

> sim-fpu.c contains other changes (comments added, lines removed).  If you think the changes belong with this patch, mention them here, otherwise submit them as a separate patch.

I just removed them, they are noise.  I saw them when I reviewed the patch
but was not sure if it would be an issue to keep them.

> > @@ -1551,6 +1551,89 @@ sim_fpu_div (sim_fpu *f,
> >
> >
> >  INLINE_SIM_FPU (int)
> > +sim_fpu_rem (sim_fpu *f,
> > +	     const sim_fpu *l,
> > +	     const sim_fpu *r)
> > +{
> > +  if (sim_fpu_is_snan (l))
> > +    {
> > +      *f = *l;
> > +      f->class = sim_fpu_class_qnan;
> > +      return sim_fpu_status_invalid_snan;
> > +    }
> > +  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 (l))
> > +    {
> > +      *f = *l;
> > +      f->class = sim_fpu_class_qnan;
> > +      return 0;
> > +    }
> > +  if (sim_fpu_is_qnan (r))
> > +    {
> > +      *f = *r;
> > +      f->class = sim_fpu_class_qnan;
> > +      return 0;
> > +    }
> > +  if (sim_fpu_is_infinity (l))
> > +    {
> > +      *f = sim_fpu_qnan;
> > +      return sim_fpu_status_invalid_irx;
> > +    }
> > +  if (sim_fpu_is_zero (r))
> > +    {
> > +      *f = sim_fpu_qnan;
> > +      return sim_fpu_status_invalid_div0;
> > +    }
> > +  if (sim_fpu_is_zero (l))
> > +    {
> > +      *f = *l;
> > +      return 0;
> > +    }
> > +  if (sim_fpu_is_infinity (r))
> > +    {
> > +      *f = *l;
> > +      return 0;
> > +    }
> > +  {
> > +    sim_fpu n, tmp;
> > +
> > +    /* Remainder is calculated as l-n*r, where n is l/r rounded to the
> > +       nearest integer.  The variable n is rounded half even.  */
> > +
> > +    sim_fpu_div (&n, l, r);
> > +    sim_fpu_round_64 (&n, 0, 0);
> > +
> > +    if (n.normal_exp < -1) /* If n looks like zero just return l.  */
> > +      {
> > +	*f = *l;
> > +	return 0;
> > +      }
> > +    else if (n.class == sim_fpu_class_number
> > +	&& n.normal_exp <= (NR_FRAC_GUARD)) /* If not too large round.  */
> 
> This line should be aligned with the opening parenthesis (well, one char to the right).

Right, that does move the comment out of the 80 char range, but just the
ending '*/'.  I hope thats ok.

> > +      do_normal_round (&n, (NR_FRAC_GUARD) - n.normal_exp, sim_fpu_round_near);
> > +
> > +    /* Mark 0's as zero so multiply can detect zero.  */
> > +    if (n.fraction == 0)
> > +      n.class = sim_fpu_class_zero;
> > +
> > +    /* Calculate n*r.  */
> > +    sim_fpu_mul (&tmp, &n, r);
> > +    sim_fpu_round_64 (&tmp, 0, 0);
> > +
> > +    /* Finally calculate l-n*r.  */
> > +    sim_fpu_sub (f, l, &tmp);
> > +
> > +    return 0;
> > +  }
> > +}
> 
> Thanks,
>
> Simon

Thank you,
-Stafford
Simon Marchi - Oct. 8, 2017, 2:06 p.m.
On 2017-10-08 08:23, Stafford Horne wrote:
>> > +    if (n.normal_exp < -1) /* If n looks like zero just return l.  */
>> > +      {
>> > +	*f = *l;
>> > +	return 0;
>> > +      }
>> > +    else if (n.class == sim_fpu_class_number
>> > +	&& n.normal_exp <= (NR_FRAC_GUARD)) /* If not too large round.  */
>> 
>> This line should be aligned with the opening parenthesis (well, one 
>> char to the right).
> 
> Right, that does move the comment out of the 80 char range, but just 
> the
> ending '*/'.  I hope thats ok.

Hmm, when I try it the trailing */ arrives right before the 80th column.

Simon

Patch

diff --git a/sim/common/cgen-accfp.c b/sim/common/cgen-accfp.c
index afbca6d582..5d600c6e41 100644
--- a/sim/common/cgen-accfp.c
+++ b/sim/common/cgen-accfp.c
@@ -93,6 +93,25 @@  divsf (CGEN_FPU* fpu, SF x, SF y)
 }
 
 static SF
+remsf (CGEN_FPU* fpu, SF x, SF y)
+{
+  sim_fpu op1;
+  sim_fpu op2;
+  sim_fpu ans;
+  unsigned32 res;
+  sim_fpu_status status;
+
+  sim_fpu_32to (&op1, x);
+  sim_fpu_32to (&op2, y);
+  status = sim_fpu_rem (&ans, &op1, &op2);
+  if (status != 0)
+    (*fpu->ops->error) (fpu, status);
+  sim_fpu_to32 (&res, &ans);
+
+  return res;
+}
+
+static SF
 negsf (CGEN_FPU* fpu, SF x)
 {
   sim_fpu op1;
@@ -453,6 +472,25 @@  divdf (CGEN_FPU* fpu, DF x, DF y)
 }
 
 static DF
+remdf (CGEN_FPU* fpu, DF x, DF y)
+{
+  sim_fpu op1;
+  sim_fpu op2;
+  sim_fpu ans;
+  unsigned64 res;
+  sim_fpu_status status;
+
+  sim_fpu_64to (&op1, x);
+  sim_fpu_64to (&op2, y);
+  status = sim_fpu_rem (&ans, &op1, &op2);
+  if (status != 0)
+    (*fpu->ops->error) (fpu, status);
+  sim_fpu_to64(&res, &ans);
+
+  return res;
+}
+
+static DF
 negdf (CGEN_FPU* fpu, DF x)
 {
   sim_fpu op1;
@@ -664,6 +702,7 @@  cgen_init_accurate_fpu (SIM_CPU* cpu, CGEN_FPU* fpu, CGEN_FPU_ERROR_FN* error)
   o->subsf = subsf;
   o->mulsf = mulsf;
   o->divsf = divsf;
+  o->remsf = remsf;
   o->negsf = negsf;
   o->abssf = abssf;
   o->sqrtsf = sqrtsf;
@@ -682,6 +721,7 @@  cgen_init_accurate_fpu (SIM_CPU* cpu, CGEN_FPU* fpu, CGEN_FPU_ERROR_FN* error)
   o->subdf = subdf;
   o->muldf = muldf;
   o->divdf = divdf;
+  o->remdf = remdf;
   o->negdf = negdf;
   o->absdf = absdf;
   o->sqrtdf = sqrtdf;
diff --git a/sim/common/cgen-fpu.h b/sim/common/cgen-fpu.h
index 134b4d031c..5f9b55d32e 100644
--- a/sim/common/cgen-fpu.h
+++ b/sim/common/cgen-fpu.h
@@ -69,6 +69,7 @@  struct cgen_fp_ops {
   SF (*subsf) (CGEN_FPU*, SF, SF);
   SF (*mulsf) (CGEN_FPU*, SF, SF);
   SF (*divsf) (CGEN_FPU*, SF, SF);
+  SF (*remsf) (CGEN_FPU*, SF, SF);
   SF (*negsf) (CGEN_FPU*, SF);
   SF (*abssf) (CGEN_FPU*, SF);
   SF (*sqrtsf) (CGEN_FPU*, SF);
@@ -93,6 +94,7 @@  struct cgen_fp_ops {
   DF (*subdf) (CGEN_FPU*, DF, DF);
   DF (*muldf) (CGEN_FPU*, DF, DF);
   DF (*divdf) (CGEN_FPU*, DF, DF);
+  DF (*remdf) (CGEN_FPU*, DF, DF);
   DF (*negdf) (CGEN_FPU*, DF);
   DF (*absdf) (CGEN_FPU*, DF);
   DF (*sqrtdf) (CGEN_FPU*, DF);
@@ -142,6 +144,7 @@  struct cgen_fp_ops {
   XF (*subxf) (CGEN_FPU*, XF, XF);
   XF (*mulxf) (CGEN_FPU*, XF, XF);
   XF (*divxf) (CGEN_FPU*, XF, XF);
+  XF (*remxf) (CGEN_FPU*, XF, XF);
   XF (*negxf) (CGEN_FPU*, XF);
   XF (*absxf) (CGEN_FPU*, XF);
   XF (*sqrtxf) (CGEN_FPU*, XF);
@@ -180,6 +183,7 @@  struct cgen_fp_ops {
   TF (*subtf) (CGEN_FPU*, TF, TF);
   TF (*multf) (CGEN_FPU*, TF, TF);
   TF (*divtf) (CGEN_FPU*, TF, TF);
+  TF (*remtf) (CGEN_FPU*, TF, TF);
   TF (*negtf) (CGEN_FPU*, TF);
   TF (*abstf) (CGEN_FPU*, TF);
   TF (*sqrttf) (CGEN_FPU*, TF);
diff --git a/sim/common/sim-fpu.c b/sim/common/sim-fpu.c
index 0d4d08ae86..854d19dfd8 100644
--- a/sim/common/sim-fpu.c
+++ b/sim/common/sim-fpu.c
@@ -41,7 +41,6 @@  along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "sim-io.h"
 #include "sim-assert.h"
 
-
 /* Debugging support.
    If digits is -1, then print all digits.  */
 
@@ -834,6 +833,7 @@  do_normal_round (sim_fpu *f,
   unsigned64 guardmask = LSMASK64 (nr_guards - 1, 0);
   unsigned64 guardmsb = LSBIT64 (nr_guards - 1);
   unsigned64 fraclsb = guardmsb << 1;
+  /* If there are decimal values do the round.  */
   if ((f->fraction & guardmask))
     {
       int status = sim_fpu_status_inexact;
@@ -865,6 +865,7 @@  do_normal_round (sim_fpu *f,
 	case sim_fpu_round_zero:
 	  break;
 	}
+      /* Actually do the rounding by masking out the decimals.  */
       f->fraction &= ~guardmask;
       /* Round if needed, handle resulting overflow.  */
       if ((status & sim_fpu_status_rounded))
@@ -1384,7 +1385,6 @@  sim_fpu_mul (sim_fpu *f,
        up in the high 64 bit word.  In the source the decimal point
        was at NR_FRAC_GUARD. */
     f->normal_exp += NR_FRAC_GUARD + 64 - (NR_FRAC_GUARD * 2);
-
     /* The high word is bounded according to the above.  Consequently
        it has never overflowed into IMPLICIT_2. */
     ASSERT (high < LSBIT64 (((NR_FRAC_GUARD + 1) * 2) - 64));
@@ -1551,6 +1551,89 @@  sim_fpu_div (sim_fpu *f,
 
 
 INLINE_SIM_FPU (int)
+sim_fpu_rem (sim_fpu *f,
+	     const sim_fpu *l,
+	     const sim_fpu *r)
+{
+  if (sim_fpu_is_snan (l))
+    {
+      *f = *l;
+      f->class = sim_fpu_class_qnan;
+      return sim_fpu_status_invalid_snan;
+    }
+  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 (l))
+    {
+      *f = *l;
+      f->class = sim_fpu_class_qnan;
+      return 0;
+    }
+  if (sim_fpu_is_qnan (r))
+    {
+      *f = *r;
+      f->class = sim_fpu_class_qnan;
+      return 0;
+    }
+  if (sim_fpu_is_infinity (l))
+    {
+      *f = sim_fpu_qnan;
+      return sim_fpu_status_invalid_irx;
+    }
+  if (sim_fpu_is_zero (r))
+    {
+      *f = sim_fpu_qnan;
+      return sim_fpu_status_invalid_div0;
+    }
+  if (sim_fpu_is_zero (l))
+    {
+      *f = *l;
+      return 0;
+    }
+  if (sim_fpu_is_infinity (r))
+    {
+      *f = *l;
+      return 0;
+    }
+  {
+    sim_fpu n, tmp;
+
+    /* Remainder is calculated as l-n*r, where n is l/r rounded to the
+       nearest integer.  The variable n is rounded half even.  */
+
+    sim_fpu_div (&n, l, r);
+    sim_fpu_round_64 (&n, 0, 0);
+
+    if (n.normal_exp < -1) /* If n looks like zero just return l.  */
+      {
+	*f = *l;
+	return 0;
+      }
+    else if (n.class == sim_fpu_class_number
+	&& n.normal_exp <= (NR_FRAC_GUARD)) /* If not too large round.  */
+      do_normal_round (&n, (NR_FRAC_GUARD) - n.normal_exp, sim_fpu_round_near);
+
+    /* Mark 0's as zero so multiply can detect zero.  */
+    if (n.fraction == 0)
+      n.class = sim_fpu_class_zero;
+
+    /* Calculate n*r.  */
+    sim_fpu_mul (&tmp, &n, r);
+    sim_fpu_round_64 (&tmp, 0, 0);
+
+    /* Finally calculate l-n*r.  */
+    sim_fpu_sub (f, l, &tmp);
+
+    return 0;
+  }
+}
+
+
+INLINE_SIM_FPU (int)
 sim_fpu_max (sim_fpu *f,
 	     const sim_fpu *l,
 	     const sim_fpu *r)
@@ -2533,6 +2616,9 @@  sim_fpu_print_status (int status,
 	case sim_fpu_status_invalid_sqrt:
 	  print (arg, "%sSQRT", prefix);
 	  break;
+	case sim_fpu_status_invalid_irx:
+	  print (arg, "%sIRX", prefix);
+	  break;
 	case sim_fpu_status_inexact:
 	  print (arg, "%sX", prefix);
 	  break;
diff --git a/sim/common/sim-fpu.h b/sim/common/sim-fpu.h
index d27d80a513..adf3b1904a 100644
--- a/sim/common/sim-fpu.h
+++ b/sim/common/sim-fpu.h
@@ -146,11 +146,12 @@  typedef enum
   sim_fpu_status_invalid_div0 = 128, /* (X / 0) */
   sim_fpu_status_invalid_cmp = 256, /* compare */
   sim_fpu_status_invalid_sqrt = 512,
-  sim_fpu_status_rounded = 1024,
-  sim_fpu_status_inexact = 2048,
-  sim_fpu_status_overflow = 4096,
-  sim_fpu_status_underflow = 8192,
-  sim_fpu_status_denorm = 16384,
+  sim_fpu_status_invalid_irx = 1024, /* (inf % X) */
+  sim_fpu_status_rounded = 2048,
+  sim_fpu_status_inexact = 4096,
+  sim_fpu_status_overflow = 8192,
+  sim_fpu_status_underflow = 16384,
+  sim_fpu_status_denorm = 32768,
 } sim_fpu_status;
 
 
@@ -230,6 +231,8 @@  INLINE_SIM_FPU (int) sim_fpu_mul (sim_fpu *f,
 				  const sim_fpu *l, const sim_fpu *r);
 INLINE_SIM_FPU (int) sim_fpu_div (sim_fpu *f,
 				  const sim_fpu *l, const sim_fpu *r);
+INLINE_SIM_FPU (int) sim_fpu_rem (sim_fpu *f,
+				  const sim_fpu *l, const sim_fpu *r);
 INLINE_SIM_FPU (int) sim_fpu_max (sim_fpu *f,
 				  const sim_fpu *l, const sim_fpu *r);
 INLINE_SIM_FPU (int) sim_fpu_min (sim_fpu *f,