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

[Nobuhiro Iwamatsu <iwamatsu at nigauri dot org>]

Hi,

Thnak you a lot.

Best regards,
  Nobuhiro

2012/6/10 Thomas Schwinge <thomas@codesourcery.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



-- 
Nobuhiro Iwamatsu
?? iwamatsu at {nigauri.org / debian.org}
?? GPG ID: 40AD1FA6