[simplify.c] bug?

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[simplify.c] bug?

Jerry DeLisle
While playing with implementing simplify for atanh function I noticed
the following:

$ cat asin.f90
program test_asin
     real(8) :: x = .5
     real(8) :: y
     y = asin(1.1)
     print *, x, y, sin(y)
end program test_asin

$ gfc asin.f90
  In file asin.f90:4

     y = asin(1.1)
             1
Error: Argument of ASIN at (1) must be between -1 and 1

Using asin(1.1_4) correctly gives the same error message.

However asin(1.1_8) is not caught evidently because 1.1_8 is not
determined to be an EXPR_CONSTANT. So, gfc_simplify_asin exits before
the test for out of range is done and the constant is not simplfied further.

The result returns NaN at run time appropriately, but the error is not
caught at compile time similar to 1.1_4.

Should 1.1_8 give the same result as 1.1_4 at compile time?

Jerry
Reply | Threaded
Open this post in threaded view
|

Re: [simplify.c] bug?

Steve Kargl
On Sat, Jun 11, 2005 at 10:09:52PM -0700, Jerry DeLisle wrote:
> While playing with implementing simplify for atanh function I noticed

If you implement atanh in a similar fashion as besj0 or erfc,
there is no simplification routine.  Simplification does more
than just constant folding.  Are you implementing atanh to take
an array argument and return an array of results?

>
> Should 1.1_8 give the same result as 1.1_4 at compile time?
>

Could be a bug?  I'll look at this later today.

--
Steve
Reply | Threaded
Open this post in threaded view
|

Re: [simplify.c] bug?

Steve Kargl
On Sun, Jun 12, 2005 at 07:39:07AM -0700, Steve Kargl wrote:

> On Sat, Jun 11, 2005 at 10:09:52PM -0700, Jerry DeLisle wrote:
> > While playing with implementing simplify for atanh function I noticed
>
> If you implement atanh in a similar fashion as besj0 or erfc,
> there is no simplification routine.  Simplification does more
> than just constant folding.  Are you implementing atanh to take
> an array argument and return an array of results?
>
> >
> > Should 1.1_8 give the same result as 1.1_4 at compile time?
> >
>
> Could be a bug?  I'll look at this later today.
>

I instrumented gfc_simplify_asin() with

int i,j;

  mpfr_out_str (stderr, 10, 0, x->value.real, GFC_RND_MODE);
  i = mpfr_cmp_si (x->value.real, 1);
  j = mpfr_cmp_si (x->value.real, -1);
  printf("\ni = %d,  j = %d\n", i, j);

  if (i > 0 || j < 0)
    {
      printf("here\n");
      gfc_error ("Argument of ASIN at %L must be between -1 and 1", &x->where);
      printf("there\n");
      return &gfc_bad_expr;
      printf("where\n");
    }

which gives

kargl[214] gfc41 -static -o z asin.f90
1.10000000000000
i = 1,  j = 1
here
there
kargl[217] ./z
                     NaN

So &gfc_bad_expr appears to be set to something that permits
compilation to continue without an error.

--
Steve
Reply | Threaded
Open this post in threaded view
|

Re: [simplify.c] bug?

Jerry DeLisle
In reply to this post by Steve Kargl
Steve Kargl wrote:
> If you implement atanh in a similar fashion as besj0 or erfc,
> there is no simplification routine.  Simplification does more
> than just constant folding.  Are you implementing atanh to take
> an array argument and return an array of results?
>
Yes, I am following besj0.  I was exploring checking for range limits
with atanh when I stumbled on the odd behavior.  I used asin as an
example for the list because the other is still cooking.

Thanks,

Jerry
Reply | Threaded
Open this post in threaded view
|

Re: [simplify.c] bug?

Jerry DeLisle
In reply to this post by Steve Kargl
Steve Kargl wrote:

> I instrumented gfc_simplify_asin() with
>
> int i,j;
>
>   mpfr_out_str (stderr, 10, 0, x->value.real, GFC_RND_MODE);
>   i = mpfr_cmp_si (x->value.real, 1);
>   j = mpfr_cmp_si (x->value.real, -1);
>   printf("\ni = %d,  j = %d\n", i, j);
>
>   if (i > 0 || j < 0)
>     {
>       printf("here\n");
>       gfc_error ("Argument of ASIN at %L must be between -1 and 1", &x->where);
>       printf("there\n");
>       return &gfc_bad_expr;
>       printf("where\n");
>     }
>
> which gives
>
> kargl[214] gfc41 -static -o z asin.f90
> 1.10000000000000
> i = 1,  j = 1
> here
> there
> kargl[217] ./z
>                      NaN
>
> So &gfc_bad_expr appears to be set to something that permits
> compilation to continue without an error.
>

gfc_error did not print either from your output so gfc_suppress_error
must be getting set somewhere?

Jerry
Reply | Threaded
Open this post in threaded view
|

Re: [simplify.c] bug?

Steve Kargl
In reply to this post by Jerry DeLisle
On Sun, Jun 12, 2005 at 08:51:09AM -0700, Jerry DeLisle wrote:

> Steve Kargl wrote:
> >If you implement atanh in a similar fashion as besj0 or erfc,
> >there is no simplification routine.  Simplification does more
> >than just constant folding.  Are you implementing atanh to take
> >an array argument and return an array of results?
> >
> Yes, I am following besj0.  I was exploring checking for range limits
> with atanh when I stumbled on the odd behavior.  I used asin as an
> example for the list because the other is still cooking.
>

AFAICT, the simplification functions are working as designed.
It appears the error message is not properly propagated back
through the call chain.

Okay, I know wants going on (sort of).

Consider the three programs

program asin_specific
   real(4) x
   x = asin(1.1)
end program asin_specific

program dasin_specific
   real(8) x
   x = dasin(1.1_8)
end program dasin_specific

program asin_generic
   real(8) x
   x = asin(1.1_8)
end program asin_generic

The first 2 program yield errors from the simplification
because this portion


  if (expr->value.function.isym != NULL)
    return (do_simplify (expr->value.function.isym, expr) == FAILURE)
      ? MATCH_ERROR : MATCH_YES;

of gfc_intrinsic_func_interface() in intrinsic.c is executed.

For the case of a generic function, expr->value.function.isym == NULL
and by pass this chunk.  I need to check the rest of
gfc_intrinsic_func_interface() to see how failures with generics are
handled.


--
Steve
Reply | Threaded
Open this post in threaded view
|

gfortran PATCH was Re: [simplify.c] bug?

Steve Kargl
On Sun, Jun 12, 2005 at 09:25:45AM -0700, Steve Kargl wrote:

> On Sun, Jun 12, 2005 at 08:51:09AM -0700, Jerry DeLisle wrote:
> > Steve Kargl wrote:
> > >If you implement atanh in a similar fashion as besj0 or erfc,
> > >there is no simplification routine.  Simplification does more
> > >than just constant folding.  Are you implementing atanh to take
> > >an array argument and return an array of results?
> > >
> > Yes, I am following besj0.  I was exploring checking for range limits
> > with atanh when I stumbled on the odd behavior.  I used asin as an
> > example for the list because the other is still cooking.
> >
>
> AFAICT, the simplification functions are working as designed.
> It appears the error message is not properly propagated back
> through the call chain.
>
> Okay, I know wants going on (sort of).
>
> Consider the three programs
>
> program asin_specific
>    real(4) x
>    x = asin(1.1)
> end program asin_specific
>
> program dasin_specific
>    real(8) x
>    x = dasin(1.1_8)
> end program dasin_specific
>
> program asin_generic
>    real(8) x
>    x = asin(1.1_8)
> end program asin_generic
>
> The first 2 program yield errors from the simplification
> because this portion
>
>
>   if (expr->value.function.isym != NULL)
>     return (do_simplify (expr->value.function.isym, expr) == FAILURE)
>       ? MATCH_ERROR : MATCH_YES;
>
> of gfc_intrinsic_func_interface() in intrinsic.c is executed.
>
> For the case of a generic function, expr->value.function.isym == NULL
> and by pass this chunk.  I need to check the rest of
> gfc_intrinsic_func_interface() to see how failures with generics are
> handled.
>
The attached patch enables the reporting of errors for
generic functions whose simplification routine returns
FAILURE.  That is, the above asin_generic program will
cause gfortran to issue

kargl[251] gfc41 -static -o z asin.f90
 In file asin.f90:3

   y = asin(1.1_8)
           1
Error: Argument of ASIN at (1) must be between -1 and 1

2005-06-12  Steven G. Kargl  <[hidden email]>

        * intrinsic.c (gfc_intrinsic_func_interface): Enable errors for generic
        functions whose simplification routine return FAILURE.

--
Steve

intrinsic.c.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

PING Re: gfortran PATCH was Re: [simplify.c] bug?

Steve Kargl

PING.

On Sun, Jun 12, 2005 at 10:28:20AM -0700, Steve Kargl wrote:

> The attached patch enables the reporting of errors for
> generic functions whose simplification routine returns
> FAILURE.  That is, the above asin_generic program will
> cause gfortran to issue
>
> kargl[251] gfc41 -static -o z asin.f90
>  In file asin.f90:3
>
>    y = asin(1.1_8)
>            1
> Error: Argument of ASIN at (1) must be between -1 and 1
>
> 2005-06-12  Steven G. Kargl  <[hidden email]>
>
> * intrinsic.c (gfc_intrinsic_func_interface): Enable errors for generic
> functions whose simplification routine return FAILURE.
>
> Index: intrinsic.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/fortran/intrinsic.c,v
> retrieving revision 1.47
> diff -c -p -r1.47 intrinsic.c
> *** intrinsic.c 1 Jun 2005 19:17:32 -0000 1.47
> --- intrinsic.c 12 Jun 2005 17:20:32 -0000
> *************** got_specific:
> *** 3007,3017 ****
>     expr->value.function.isym = specific;
>     gfc_intrinsic_symbol (expr->symtree->n.sym);
>  
>     if (do_simplify (specific, expr) == FAILURE)
> !     {
> !       gfc_suppress_error = 0;
> !       return MATCH_ERROR;
> !     }
>  
>     /* TODO: We should probably only allow elemental functions here.  */
>     flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);
> --- 3007,3015 ----
>     expr->value.function.isym = specific;
>     gfc_intrinsic_symbol (expr->symtree->n.sym);
>  
> +   gfc_suppress_error = 0;
>     if (do_simplify (specific, expr) == FAILURE)
> !     return MATCH_ERROR;
>  
>     /* TODO: We should probably only allow elemental functions here.  */
>     flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);


--
Steve
Reply | Threaded
Open this post in threaded view
|

Re: PING Re: gfortran PATCH was Re: [simplify.c] bug?

Steven Bosscher
On Saturday 18 June 2005 06:35, Steve Kargl wrote:

> PING.
>
> On Sun, Jun 12, 2005 at 10:28:20AM -0700, Steve Kargl wrote:
> > The attached patch enables the reporting of errors for
> > generic functions whose simplification routine returns
> > FAILURE.  That is, the above asin_generic program will
> > cause gfortran to issue
> >
> > kargl[251] gfc41 -static -o z asin.f90
> >  In file asin.f90:3
> >
> >    y = asin(1.1_8)
> >            1
> > Error: Argument of ASIN at (1) must be between -1 and 1
> >
> > 2005-06-12  Steven G. Kargl  <[hidden email]>
> >
> > * intrinsic.c (gfc_intrinsic_func_interface): Enable errors for generic
> > functions whose simplification routine return FAILURE.
> >
> > Index: intrinsic.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/fortran/intrinsic.c,v
> > retrieving revision 1.47
> > diff -c -p -r1.47 intrinsic.c
> > *** intrinsic.c 1 Jun 2005 19:17:32 -0000 1.47
> > --- intrinsic.c 12 Jun 2005 17:20:32 -0000
> > *************** got_specific:
> > *** 3007,3017 ****
> >     expr->value.function.isym = specific;
> >     gfc_intrinsic_symbol (expr->symtree->n.sym);
> >
> >     if (do_simplify (specific, expr) == FAILURE)
> > !     {
> > !       gfc_suppress_error = 0;
> > !       return MATCH_ERROR;
> > !     }
> >
> >     /* TODO: We should probably only allow elemental functions here.  */
> >     flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);
> >     --- 3007,3015 ----
> >     expr->value.function.isym = specific;
> >     gfc_intrinsic_symbol (expr->symtree->n.sym);
> >
> > +   gfc_suppress_error = 0;
> >     if (do_simplify (specific, expr) == FAILURE)
> > !     return MATCH_ERROR;
> >
> >     /* TODO: We should probably only allow elemental functions here.  */
> >     flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);

A bigger chunk of the code you are patching:

got_specific:
  expr->value.function.isym = specific;
  gfc_intrinsic_symbol (expr->symtree->n.sym);

  if (do_simplify (specific, expr) == FAILURE)
    {
      gfc_suppress_error = 0;
      return MATCH_ERROR;
    }

  /* TODO: We should probably only allow elemental functions here.  */
  flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);

  gfc_suppress_error = 0;
  if (pedantic && gfc_init_expr


After your patch it looks like this:

got_specific:
  expr->value.function.isym = specific;
  gfc_intrinsic_symbol (expr->symtree->n.sym);

  gfc_suppress_error = 0;
  if (do_simplify (specific, expr) == FAILURE)
    return MATCH_ERROR;

  /* TODO: We should probably only allow elemental functions here.  */
  flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);

  gfc_suppress_error = 0;
  if (pedantic && gfc_init_expr


I would hope do_simplify cannot change gfc_suppress_error, the second
"gfc_suppress_error = 0;" should now be redundant.  Can you check that
please?

The patch looks OK to me otherwise.

Gr.
Steven

Reply | Threaded
Open this post in threaded view
|

Re: PING Re: gfortran PATCH was Re: [simplify.c] bug?

Steve Kargl
On Sat, Jun 18, 2005 at 12:17:06PM +0200, Steven Bosscher wrote:

>
>   gfc_suppress_error = 0;
>   if (do_simplify (specific, expr) == FAILURE)
>     return MATCH_ERROR;
>
>   /* TODO: We should probably only allow elemental functions here.  */
>   flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);
>
>   gfc_suppress_error = 0;
>   if (pedantic && gfc_init_expr
>
>
> I would hope do_simplify cannot change gfc_suppress_error, the second
> "gfc_suppress_error = 0;" should now be redundant.  Can you check that
> please?

You are correct.  I noticed the extra gfc_suppress_error = 0 after
I cut the original diff.  I was going to delete the extra one
under the "obviously correct" rule.

--
Steve
Reply | Threaded
Open this post in threaded view
|

Re: PING Re: gfortran PATCH was Re: [simplify.c] bug?

Steve Kargl
On Sat, Jun 18, 2005 at 09:37:47AM -0700, Steve Kargl wrote:

> On Sat, Jun 18, 2005 at 12:17:06PM +0200, Steven Bosscher wrote:
> >
> >   gfc_suppress_error = 0;
> >   if (do_simplify (specific, expr) == FAILURE)
> >     return MATCH_ERROR;
> >
> >   /* TODO: We should probably only allow elemental functions here.  */
> >   flag |= (expr->ts.type != BT_INTEGER && expr->ts.type != BT_CHARACTER);
> >
> >   gfc_suppress_error = 0;
> >   if (pedantic && gfc_init_expr
> >
> >
> > I would hope do_simplify cannot change gfc_suppress_error, the second
> > "gfc_suppress_error = 0;" should now be redundant.  Can you check that
> > please?
>
> You are correct.  I noticed the extra gfc_suppress_error = 0 after
> I cut the original diff.  I was going to delete the extra one
> under the "obviously correct" rule.


Committed to 4.1.  I'll commit it to 4.0 when it re-opens.

--
Steve