User account creation filtered due to spam.

Bug 43040 - Wrong decl for mathbuiltins -> wrong code with LTO
Summary: Wrong decl for mathbuiltins -> wrong code with LTO
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Francois-Xavier Coudert
URL:
Keywords: lto, wrong-code
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2010-02-11 17:44 UTC by Tobias Burnus
Modified: 2010-06-09 10:20 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-06-08 20:39:06


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2010-02-11 17:44:24 UTC
gfortran -g -flto gfortran.dg/gamma_5.f90 fails when calculating gamma.

Simplified version below. Without -flto it prints:
           1   1.5000000      0.88622695
while with -flto it shows:
           1   1.5000000     -0.12078223

  integer :: n
  real :: xs
  n = 1
  xs = n + 0.5
  print *, n, xs, gamma(xs)
  end
Comment 1 Tobias Burnus 2010-02-11 18:06:35 UTC
Dump (w/ putting "xs" on an extra line):

  n = 1;
  xs = (real(kind=4)) n + 5.0e-1;
  xs = __builtin_tgammaf (xs);
Comment 2 Richard Biener 2010-02-12 11:07:37 UTC
This is because the Fortran frontend has different DECL_FUNCTION_CODE for the
same builtins as the middle-end, so they get mixed up (in this case LGAMMA
and GAMMA).

This needs to be fixed.

I also see

  /* We define these separately as the fortran versions have different
     semantics (they return an integer type) */
  gfc_define_builtin ("__builtin_roundl", mfunc_longdouble[0],
                      BUILT_IN_ROUNDL, "roundl", true);
...

ugh.  You can't overload existing builtin names with different semantics.
The middle-end expects that of gcc/builtins.def.  For the above case
there exists BUILT_IN_LROUNDL.
Comment 3 Tobias Burnus 2010-02-12 13:54:56 UTC
(In reply to comment #2)
>   /* We define these separately as the fortran versions have different
>      semantics (they return an integer type) */
>   gfc_define_builtin ("__builtin_roundl", mfunc_longdouble[0],
> 
> ugh.  You can't overload existing builtin names with different semantics.
> The middle-end expects that of gcc/builtins.def.  For the above case
> there exists BUILT_IN_LROUNDL.

My impression is that the comment is old and does not apply to "roundl" as the trans-intrinsics.c uses both BUILT_IN_ROUND* and BUILT_IN_LROUND - and given that the "round*" were added much later than the comment according to svn blame.

Having said that, I think one should check the other intrinsics in that file (f95-lang.c) and remove then the bogus comment.
Comment 4 Tobias Burnus 2010-02-16 08:35:19 UTC
Subject: Bug 43040

Author: burnus
Date: Tue Feb 16 08:35:05 2010
New Revision: 156796

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156796
Log:
2010-02-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/43040
        * gfortran.h (gfc_isym_id): Rename GFS_ISYM_GAMMA to
        * GFS_ISYM_TGAMMA.
        * intrinsic.c (add_functions): Ditto.
        * iresolve.c (gfc_resolve_gamma): Call tgamma instead of gamma.
        * mathbuiltins.def: Use TGAMMA instead of GAMMA with "tgamma".


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/fortran/mathbuiltins.def

Comment 5 Tobias Burnus 2010-02-16 13:51:23 UTC
I think the comment applies only to FLOOR and ROUND - at least when it was first written those were the only ones present. If I read the source code correctly, the ME floor(l,f) is not directly called but instead the following is generated:
  FLOOR(x) = INT(x) <= x ? INT(x) : INT(x) - 1
cf. trans-intrinsic.c. I also believe that ROUND [(l)round(l,f)] is properly handled, which means that the PR could be closed. However, I leave it open for someone else to cross check.
Comment 6 Francois-Xavier Coudert 2010-06-08 20:39:06 UTC
I'm currently looking at math builtins for __float128 support, so I'll check that.
Comment 7 Francois-Xavier Coudert 2010-06-09 10:18:08 UTC
Subject: Bug 43040

Author: fxcoudert
Date: Wed Jun  9 10:17:56 2010
New Revision: 160459

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=160459
Log:
	PR fortran/43040
	* f95-lang.c (gfc_init_builtin_functions): Remove comment.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/f95-lang.c

Comment 8 Francois-Xavier Coudert 2010-06-09 10:20:22 UTC
Checked that it is not an issue.