Re: AC_C_BIGENDIAN answers "universal" on powerpc-aix

[Ralf Wildenhues]

Hello Peter, all,

sorry for the delay in reviewing:

* Peter O'Gorman wrote on Thu, Aug 14, 2008 at 11:52:07PM CEST:
> >From f58daa236229e04e0799c20a131ff88e5e996789 Mon Sep 17 00:00:00 2001
> From: Peter O'Gorman <address@hidden>
> Date: Thu, 14 Aug 2008 16:04:30 -0500
> Subject: [PATCH] Limit AC_C_BIGENDIAN univeral checks to Mac OS X.
> 
> * lib/autoconf/c.m4 (AC_C_BIGENDIAN): Limit the check to __APPLE_CC__ with 
> possible -arch flags.
> * NEWS: Document it.

> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,10 @@ GNU Autoconf NEWS - User visible changes.
>  
>  * Major changes in Autoconf 2.62a (2008-??-??)
>  
> +** AC_C_BIGENDIAN does not mistakenly report "universal" for some
> +   bigendian hosts, a regression introduced with universal binary
> +   support in 2.62.
> +

What about the case
  ./configure CC="gcc -arch ppc"

i.e., where only one -arch is given?  The current code detects that as
universal, but the manual isn't clear that that is what should happen.
What do you think would be best?  Ahh, rereading the thread, I gather
we've already been there, and having the answer be "universal" for this
case on Darwin isn't much of a problem.  OK.

> --- a/lib/autoconf/c.m4
> +++ b/lib/autoconf/c.m4
> @@ -1399,11 +1399,15 @@ fi
>  AC_DEFUN([AC_C_BIGENDIAN],
>  [AH_VERBATIM([WORDS_BIGENDIAN],
>  [/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most
> -   significant byte first (like Motorola and SPARC, unlike Intel and VAX). */
> -#if defined __BIG_ENDIAN__
> -# define WORDS_BIGENDIAN 1
> -#elif ! defined __LITTLE_ENDIAN__
> -# undef WORDS_BIGENDIAN
> +   significant byte first (like Motorola and SPARC, unlike Intel). */
> +#if defined AC_APPLE_UNIVERSAL_BUILD
> +# if defined __BIG_ENDIAN__
> +#  define WORDS_BIGENDIAN 1
> +# endif
> +#else
> +# ifndef WORDS_BIGENDIAN
> +#  undef WORDS_BIGENDIAN
> +# endif
>  #endif])dnl
>   AC_CACHE_CHECK([whether byte ordering is bigendian], [ac_cv_c_bigendian],
>     [ac_cv_c_bigendian=unknown
> @@ -1411,12 +1415,18 @@ AC_DEFUN([AC_C_BIGENDIAN],
>        [# See if __BIG_ENDIAN__ or __LITTLE_ENDIAN__ is defined.

Your patch makes this comment wrong.  How about "See if we're dealing
with a universal compiler."?

>         AC_COMPILE_IFELSE(
>        [AC_LANG_SOURCE(
> -         [[#if ! (defined __BIG_ENDIAN__ || defined __LITTLE_ENDIAN__)
> -            neither is defined;
> +         [[#ifndef __APPLE_CC__
> +            not a universal capable compiler 

Please remove trailing whitespace.

>            #endif
>            typedef int dummy;
>           ]])],
> -      [ac_cv_c_bigendian=universal])],
> +      [
> +     # Check for potential -arch flags it is not universal unless

Nit picks:  s/flags it/flags.  It/

> +     # there are some -arch flags, note that *ppc* also matches

s/flags, note/flags.  Note/

> +     # ppc64. This check is also rather less than ideal.

Again two spaces after period.

> +     case "${CC} ${CFLAGS} ${CPPFLAGS} ${LDFLAGS}" in  #(
> +       *-arch*ppc*|*-arch*i386*|*-arch*x86_64*) ac_cv_c_bigendian=universal;;
> +     esac])],
>        [AC_DIAGNOSE([obsolete], [AC_C_BIGENDIAN suggests AC_CONFIG_HEADERS])])
>      if test $ac_cv_c_bigendian = unknown; then
>        # See if sys/param.h defines the BYTE_ORDER macro.
> @@ -1521,6 +1531,8 @@ AC_DEFUN([AC_C_BIGENDIAN],
>     no)
>       $2 ;; #(
>     universal)
> +     AC_DEFINE([AC_APPLE_UNIVERSAL_BUILD],1,
> +     [Define if building universal (internal helper macro)])
>       $4 ;; #(

These four lines need to be
   universal)
     m4_default([$4],
       [AC_DEFINE([AC_APPLE_UNIVERSAL_BUILD],1,
          [Define if building universal (internal helper macro)])])
     ;; #(

so that, if the user specifies a different ACTION-IF-UNIVERSAL, the
feature is not enabled.  At least that is how the manual documents
this.

>     *)
>       m4_default([$3],

So far I have tested Darwin/ppc (the compiler cannot produce Intel code,
however, due to installation issues), Linux/x86, Linux/parisc.  If you
think we need to test Solaris, OSF/1, then I can do that, but your patch
shouldn't change much for these.

Please commit with above changes, or tell me to do so for you, if you
agree with things.

Thanks,
Ralf