newlib: libm: Fix RISCV feraiseexcept return value when success

Message ID 20241024072645.634748-1-william.tsai@sifive.com
State New
Headers
Series newlib: libm: Fix RISCV feraiseexcept return value when success |

Commit Message

William Tsai Oct. 24, 2024, 7:26 a.m. UTC
  According to the document, feraiseexcept should return 0 when exception
is raised succesfully. The return statement is missing here causing it
always return a non-zero value even when success.
---
 newlib/libm/machine/riscv/feraiseexcept.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Corinna Vinschen Oct. 24, 2024, 8:22 a.m. UTC | #1
On Oct 24 00:26, William Tsai wrote:
> According to the document, feraiseexcept should return 0 when exception
> is raised succesfully. The return statement is missing here causing it
> always return a non-zero value even when success.
> ---
>  newlib/libm/machine/riscv/feraiseexcept.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/newlib/libm/machine/riscv/feraiseexcept.c b/newlib/libm/machine/riscv/feraiseexcept.c
> index 817fa6274..99bacd556 100644
> --- a/newlib/libm/machine/riscv/feraiseexcept.c
> +++ b/newlib/libm/machine/riscv/feraiseexcept.c
> @@ -64,6 +64,7 @@ int feraiseexcept(int excepts)
>    /* Set the requested exception flags */
>  
>    asm volatile("csrs fflags, %0" : : "r"(excepts));
> +  return 0;
>  
>    /* Per 'feraiseexcept.html:
>     * "If the argument is zero or if all the specified exceptions were
> -- 
> 2.37.1

What about the return statement in line 85, following the longish comment?


Corinna
  
William Tsai Oct. 24, 2024, 8:27 a.m. UTC | #2
The return in line 85 outside the macro, which is used when hardware does
not have floating point exception support. The added return is inside the
macro after the exception is raised.

On Thu, Oct 24, 2024 at 4:22 PM Corinna Vinschen <vinschen@redhat.com>
wrote:

> On Oct 24 00:26, William Tsai wrote:
> > According to the document, feraiseexcept should return 0 when exception
> > is raised succesfully. The return statement is missing here causing it
> > always return a non-zero value even when success.
> > ---
> >  newlib/libm/machine/riscv/feraiseexcept.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/newlib/libm/machine/riscv/feraiseexcept.c
> b/newlib/libm/machine/riscv/feraiseexcept.c
> > index 817fa6274..99bacd556 100644
> > --- a/newlib/libm/machine/riscv/feraiseexcept.c
> > +++ b/newlib/libm/machine/riscv/feraiseexcept.c
> > @@ -64,6 +64,7 @@ int feraiseexcept(int excepts)
> >    /* Set the requested exception flags */
> >
> >    asm volatile("csrs fflags, %0" : : "r"(excepts));
> > +  return 0;
> >
> >    /* Per 'feraiseexcept.html:
> >     * "If the argument is zero or if all the specified exceptions were
> > --
> > 2.37.1
>
> What about the return statement in line 85, following the longish comment?
>
>
> Corinna
>
>
  
Corinna Vinschen Oct. 25, 2024, 9:36 a.m. UTC | #3
On Oct 24 16:27, William Tsai wrote:
> The return in line 85 outside the macro, which is used when hardware does
> not have floating point exception support. The added return is inside the
> macro after the exception is raised.

Ok, I defer to the RISCV guys.  The comment following the asm statement
sounds weirdly like the non-0 return value is by choice.

Kito, can you please chime in here?


Thanks,
Corinna



> 
> On Thu, Oct 24, 2024 at 4:22 PM Corinna Vinschen <vinschen@redhat.com>
> wrote:
> 
> > On Oct 24 00:26, William Tsai wrote:
> > > According to the document, feraiseexcept should return 0 when exception
> > > is raised succesfully. The return statement is missing here causing it
> > > always return a non-zero value even when success.
> > > ---
> > >  newlib/libm/machine/riscv/feraiseexcept.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/newlib/libm/machine/riscv/feraiseexcept.c
> > b/newlib/libm/machine/riscv/feraiseexcept.c
> > > index 817fa6274..99bacd556 100644
> > > --- a/newlib/libm/machine/riscv/feraiseexcept.c
> > > +++ b/newlib/libm/machine/riscv/feraiseexcept.c
> > > @@ -64,6 +64,7 @@ int feraiseexcept(int excepts)
> > >    /* Set the requested exception flags */
> > >
> > >    asm volatile("csrs fflags, %0" : : "r"(excepts));
> > > +  return 0;
> > >
> > >    /* Per 'feraiseexcept.html:
> > >     * "If the argument is zero or if all the specified exceptions were
> > > --
> > > 2.37.1
> >
> > What about the return statement in line 85, following the longish comment?
> >
> >
> > Corinna
> >
> >
  

Patch

diff --git a/newlib/libm/machine/riscv/feraiseexcept.c b/newlib/libm/machine/riscv/feraiseexcept.c
index 817fa6274..99bacd556 100644
--- a/newlib/libm/machine/riscv/feraiseexcept.c
+++ b/newlib/libm/machine/riscv/feraiseexcept.c
@@ -64,6 +64,7 @@  int feraiseexcept(int excepts)
   /* Set the requested exception flags */
 
   asm volatile("csrs fflags, %0" : : "r"(excepts));
+  return 0;
 
   /* Per 'feraiseexcept.html:
    * "If the argument is zero or if all the specified exceptions were