Re: [PATCH 3/4 v2] [SH/SH4]: Set fpcsr register which read again

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Tue, 29 May 2012 15:02:09 +0200, I wrote:
> On Tue, 29 May 2012 14:47:14 +0200, I wrote:
> > On Thu, 24 May 2012 08:43:04 +0900, Nobuhiro Iwamatsu <iwamatsu@nigauri.org> wrote:
> > > 2012/5/23 Kaz Kojima <kkojima@rr.iij4u.or.jp>:
> > > > Thomas Schwinge <thomas@codesourcery.com> wrote:
> > > >>> + Â/* When read fpscr, this was initialized.
> > > >>> + Â Â We need to rewrite value of temp. */
> > > >>> + Â_FPU_SETCW (temp);
> > > >>>
> > > >>> Â Âreturn temp & excepts & FE_ALL_EXCEPT;
> > > >>> Â}
> > > >>
> > > >> Why is this needed? ÂIt seems that I can't find it neither in the SH-4A
> > > >> Software Manual nor in the SH7785 Hardware Manual.
> > > > 
> > > > I thought that there was an errata for the issue, though I can't
> > > > find the corresponding documentation ATM. ÂIwamatsu-san?
> > 
> > > When fetestexcept was called, the problem from which the information
> > > on fpscr is deleted occurred.
> > > To be sure, it is not writing to the manual.
> > > I checked this problem using the Linux kernel.
> > > I think that it may be a problem by the side of a Linux kernel...
> > 
> > I see.  Will you be working on resolving that?  Because, currently the
> > state is inconsistent: sometimes fpscr is reloaded, sometimes not
> > (ftestexcept, as well as (potentially) all generic uses of _FPU_GETCW
> > outside of our control).
> 
> Sorry, ftestexcept is covered, fegetround is where it is missing.
> 
> > If there indeed is an erratum regarding this, how about we directly add
> > an _FPU_SETCW call to the _FPU_GETCW macro?  This will be redundant for
> > most cases (when a different value is stored afterwards), but will at
> > least cover all usage of _FPU_GETCW.  But I'd rather prefer to first
> > clarify why/where this is really needed.  (Based on glibc's test suite,
> > it is not needed for our SH7785 board; makes no difference when reverting
> > your patch.)

I reverted the three occurrences of re-writing fpscr after reading it, to
give us the chance to resolve this properly -- and if in glibc then
consistently.

	* sysdeps/sh/sh4/fpu/fegetenv.c (fegetenv): Do not re-write fpscr after
	reading it.
	* sysdeps/sh/sh4/fpu/fegetexcept.c (fegetexcept): Likewise.
	* sysdeps/sh/sh4/fpu/ftestexcept.c (fetestexcept): Likewise.

diff --git a/sysdeps/sh/sh4/fpu/fegetenv.c b/sysdeps/sh/sh4/fpu/fegetenv.c
index ca7de16..cb3ca27 100644
--- a/sysdeps/sh/sh4/fpu/fegetenv.c
+++ b/sysdeps/sh/sh4/fpu/fegetenv.c
@@ -24,9 +24,6 @@ fegetenv (fenv_t *envp)
 {
   fpu_control_t temp;
   _FPU_GETCW (temp);
-  /* When read fpscr, this was initialized.
-     We need to rewrite value of temp. */
-  _FPU_SETCW (temp);
 
   envp->__fpscr = temp;
 
diff --git a/sysdeps/sh/sh4/fpu/fegetexcept.c b/sysdeps/sh/sh4/fpu/fegetexcept.c
index a2faaac..038fe97 100644
--- a/sysdeps/sh/sh4/fpu/fegetexcept.c
+++ b/sysdeps/sh/sh4/fpu/fegetexcept.c
@@ -27,9 +27,6 @@ fegetexcept (void)
 
   /* Get current exceptions.  */
   _FPU_GETCW (temp);
-  /* When read fpscr, this was initialized.
-     We need to rewrite value of temp. */
-  _FPU_SETCW (temp);
 
   return (temp >> 5) & FE_ALL_EXCEPT;
 }
diff --git a/sysdeps/sh/sh4/fpu/ftestexcept.c b/sysdeps/sh/sh4/fpu/ftestexcept.c
index c61af4c..8f09611 100644
--- a/sysdeps/sh/sh4/fpu/ftestexcept.c
+++ b/sysdeps/sh/sh4/fpu/ftestexcept.c
@@ -26,9 +26,6 @@ fetestexcept (int excepts)
 
   /* Get current exceptions.  */
   _FPU_GETCW (temp);
-  /* When read fpscr, this was initialized.
-     We need to rewrite value of temp. */
-  _FPU_SETCW (temp);
 
   return temp & excepts & FE_ALL_EXCEPT;
 }


GrÃÃe,
 Thomas

Attachment: pgp00000.pgp
Description: PGP signature