diff mbox

fix aix build error with math.h in gcc/sreal.c

Message ID 20141216190511.GA5043@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Dec. 16, 2014, 7:05 p.m. UTC
> On Tue, Dec 16, 2014 at 5:04 PM, Michael Haubenwallner
> <michael.haubenwallner@ssi-schaefer.com> wrote:
> > Recent commit 218765 adding sreal::to_double() breaks on AIX due to math.h
> > being included before _LARGE_FILES and __STDC_FORMAT_MACROS being defined
> > later in config.h and system.h, respectively.
> 
> sreal.c shouldn't include math.h, if really really really needed
> math.h needs to be
> included from system.h at the appropriate place.

Hmm, I need math.h for the exponential function. genautomata is also including math.h.
Should we thus move it to system.h

Something like this?

Since i do not caremuch about performance of to_double, we could also just have loop
that multiplies/divides by 2 until exponent is reached. It is rather lame though.

Honza


> 
> Richard,
> 
> > 2014-12-16  Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
> >
> >         Both config.h and system.h define ABI/API macros for system headers.
> >         * sreal.c: Include math.h later.
> >
> > Thanks!
> > /haubi/

Comments

Richard Biener Dec. 17, 2014, 9:34 a.m. UTC | #1
On Tue, Dec 16, 2014 at 8:05 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Tue, Dec 16, 2014 at 5:04 PM, Michael Haubenwallner
>> <michael.haubenwallner@ssi-schaefer.com> wrote:
>> > Recent commit 218765 adding sreal::to_double() breaks on AIX due to math.h
>> > being included before _LARGE_FILES and __STDC_FORMAT_MACROS being defined
>> > later in config.h and system.h, respectively.
>>
>> sreal.c shouldn't include math.h, if really really really needed
>> math.h needs to be
>> included from system.h at the appropriate place.
>
> Hmm, I need math.h for the exponential function. genautomata is also including math.h.
> Should we thus move it to system.h

Do we even link GCC with libm ...?  Probably as a side-effect with linking with
mpfr?  Only genautomata is explicitely linked with -lm.

Can't you simply prototype exp2() properly?  Also we don't require a
C99 runtime,
so I'm not sure that exp2 is available everywhere (didn't check the C++ standard
when it appears, but if then via <cmath>, not <math.h> I suppose - but cmath
doesn't have exp2).

Please avoid libm code in GCC, I suppose using std::pow (2., (int)m_exp) is
ok though after including <cmath> - there is a double, int overload that
uses __builtin_powi which eventually expands to a libgcc call.

Thus following genautomata I suppose we can try including <cmath> in
sreal.c after the system.h include and if that works everywhere fine.

Thanks,
Richard.

> Something like this?
>
> Since i do not caremuch about performance of to_double, we could also just have loop
> that multiplies/divides by 2 until exponent is reached. It is rather lame though.
>
> Honza
>
> Index: system.h
> ===================================================================
> --- system.h    (revision 218788)
> +++ system.h    (working copy)
> @@ -45,6 +45,8 @@
>
>  #include <stdio.h>
>
> +#include <math.h>
> +
>  /* Define a generic NULL if one hasn't already been defined.  */
>  #ifndef NULL
>  #define NULL 0
> Index: genautomata.c
> ===================================================================
> --- genautomata.c       (revision 218788)
> +++ genautomata.c       (working copy)
> @@ -113,7 +113,6 @@
>  #include "errors.h"
>  #include "gensupport.h"
>
> -#include <math.h>
>  #include "hashtab.h"
>  #include "vec.h"
>  #include "fnmatch.h"
> Index: sreal.c
> ===================================================================
> --- sreal.c     (revision 218788)
> +++ sreal.c     (working copy)
> @@ -49,7 +49,6 @@
>
>  #include "config.h"
>  #include "system.h"
> -#include <math.h>
>  #include "coretypes.h"
>  #include "sreal.h"
>
>
>>
>> Richard,
>>
>> > 2014-12-16  Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
>> >
>> >         Both config.h and system.h define ABI/API macros for system headers.
>> >         * sreal.c: Include math.h later.
>> >
>> > Thanks!
>> > /haubi/
Mike Stump Dec. 17, 2014, 10:02 p.m. UTC | #2
On Dec 17, 2014, at 1:34 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> Do we even link GCC with libm …?

Yes.  :-)  If you grew up on C++, you would realize, -lm as you call it, is just there in C++, always.  If you compile with g++ -v, you will even see it.

> Also we don't require a C99 runtime,

With the major version bump, and considering the age of a system that runs an OS from that era, I think these are mainly museum pieces.  I think it would be reasonable to add that requirement.  I think we should accept any patches from folks that want gcc to still work for whatever reason in such an environment, but, they can use libliberty, autoconfg, gnulib or some other advanced technique to make things work.  For exp2, gnulib states:

  This function is missing on some platforms: MacOS X 10.3, FreeBSD 5.2.1, NetBSD 3.0, OpenBSD 3.8, AIX 5.1, IRIX 6.5, OSF/1 4.0, Solaris 9, Interix 3.5.

to put a specific context of which systems are affected by this.  These tend to be 10 year old systems.
diff mbox

Patch

Index: system.h
===================================================================
--- system.h    (revision 218788)
+++ system.h    (working copy)
@@ -45,6 +45,8 @@ 
 
 #include <stdio.h>
 
+#include <math.h>
+
 /* Define a generic NULL if one hasn't already been defined.  */
 #ifndef NULL
 #define NULL 0
Index: genautomata.c
===================================================================
--- genautomata.c       (revision 218788)
+++ genautomata.c       (working copy)
@@ -113,7 +113,6 @@ 
 #include "errors.h"
 #include "gensupport.h"
 
-#include <math.h>
 #include "hashtab.h"
 #include "vec.h"
 #include "fnmatch.h"
Index: sreal.c
===================================================================
--- sreal.c     (revision 218788)
+++ sreal.c     (working copy)
@@ -49,7 +49,6 @@ 
 
 #include "config.h"
 #include "system.h"
-#include <math.h>
 #include "coretypes.h"
 #include "sreal.h"