AARCH64 configure check for gas -mabi support

Hi Yufeng,

Thanks for the quick response.

>> +#define ASM_MABI_SPEC    "%{mabi=lp64*:}"
> 
> Is '*' necessary here?

Removed it.

>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index b1b4eef..c1a9cbd 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -5186,6 +5186,10 @@ aarch64_override_options (void)
>>       {
>>         aarch64_parse_tune ();
>>       }
>> +#ifndef HAVE_AS_MABI_OPTION
>> +  if (TARGET_ILP32)
>> +    error ("Assembler does not supprt -mabi=ilp32");
>> +#endif
> 
> A blank line before #ifndef and some comment to explain the reason please.

Blank line and comments are added.

>> + aarch64*-*-*)
> 
> Alphabetically, this should be placed before alpha*.

Moved it up.

> 
> It is not sufficient to only check with_abi itself.  By default,
> aarch64*-*-elf builds both ilp32 and lp64 libraries (e.g. libgcc).  This
> needs to be turned off if test x$gcc_cv_as_aarch64_mabi = xno.  We also
> need to detect the situation where users explicitly configure the
> toolchain with --with-multilib-list=lp64,ilp32
> 
> Here is an incremental diff based on your change to gcc/configure.ac to
> give an example on a more thorough check:
> 
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index c8cf274..c590ad7 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -3488,12 +3488,27 @@ case "$target" in
>      gcc_GAS_CHECK_FEATURE([-mabi option],
>        gcc_cv_as_aarch64_mabi,,
>        [-mabi=lp64], [.text],,,)
> -    if test $gcc_cv_as_aarch64_mabi = yes ; then
> +    if test x$gcc_cv_as_aarch64_mabi = xyes ; then
>      AC_DEFINE(HAVE_AS_MABI_OPTION, 1,
>            [Define if your assembler supports the -mabi option.])
> -    fi
> -    if test x$gcc_cv_as_aarch64_mabi = xno && test x$with_abi = xilp32;
> then
> -    AC_MSG_ERROR([Assembler doesnot support -mabi=ilp32. Upgrade the
> Assembler.])
> +    else
> +    if test x$with_abi = xilp32; then
> +      AC_MSG_ERROR([Assembler does not support -mabi=ilp32.  Upgrade
> the Assembler.])
> +    fi
> +    if test x"$with_multilib_list" = xdefault; then
> +      TM_MULTILIB_CONFIG=lp64
> +    else
> +      aarch64_multilibs=`echo $with_multilib_list | sed -e 's/,/ /g'`
> +      for aarch64_multilib in ${aarch64_multilibs}; do
> +        case ${aarch64_multilib} in
> +          ilp32 )
> +        AC_MSG_ERROR([Assembler does not support -mabi=ilp32.  Upgrade
> the Assembler.])
> +        ;;
> +          *)
> +        ;;
> +        esac
> +      done
> +    fi
>      fi
>      ;;

Updated it and tested with

1. binutils 2.23.2
   a. bootstrapped with defaults and tested gcc for -mabi=lp64
(compiles) and -mabi=ilp32 gives error
   b. Trying to boottsrap with --with-multilibs-list=lp64,ilp32 fails
with error msg
   c. Trying to bootstrap with --with-multilibs-list=ilp32 fails with
error msg
   d. Bootstrap with --with-multilibs-list=lp64 works.

2. binutils 2.24.51
    a. bootstrapped with defaults and tested gcc for -mabi=lp64
(compiles) and -mabi=ilp32 (compiles)
   b. Bootstrap with --with-multilibs-list=lp64,ilp32 works and tested
gcc for -mabi=lp64
compiles and -mabi=ilp32  compiles(* gives linker error in my setup -
aarch64:ilp32 architecture of input file `/tmp/ccIFqSxU.o' is
incompatible with aarch64 output; I believe this is not related to what
I am testing)
   c. Bootstrap with default works


Thanks,
kugan

gcc/

+2013-12-09  Kugan Vivekanandarajah  <kuganv@linaro.org>
+	* configure.ac: Add check for aarch64 assembler -mabi support.
+	* configure: Regenerate.
+	* config.in: Regenerate.
+	* config/aarch64/aarch64-elf.h (ASM_MABI_SPEC): New define.
+	(ASM_SPEC): Update to substitute -mabi with ASM_MABI_SPEC.
+	* config/aarch64/aarch64.h (aarch64_override_options):  Issue error if
+	assebler does not support -mabi and option ilp32 is selected.
+	* doc/install.texi: Added note that building gcc 4.9 and after with pre
+	2.24 binutils will not support -mabi=ilp32.
+

Hi Kugan,

The latest patch looks good to me; I only have a couple of minor 
comments inlined below.  Please ask Marcus to review and approve it. 
Thanks again for fixing this issue!

On 12/10/13 06:21, Kugan wrote:
[snip]

> Updated it and tested with
>
> 1. binutils 2.23.2
>    a. bootstrapped with defaults and tested gcc for -mabi=lp64
> (compiles) and -mabi=ilp32 gives error
>    b. Trying to boottsrap with --with-multilibs-list=lp64,ilp32 fails
> with error msg
>    c. Trying to bootstrap with --with-multilibs-list=ilp32 fails with
> error msg
>    d. Bootstrap with --with-multilibs-list=lp64 works.
>
> 2. binutils 2.24.51
>     a. bootstrapped with defaults and tested gcc for -mabi=lp64
> (compiles) and -mabi=ilp32 (compiles)
>    b. Bootstrap with --with-multilibs-list=lp64,ilp32 works and tested
> gcc for -mabi=lp64
> compiles and -mabi=ilp32  compiles(* gives linker error in my setup -
> aarch64:ilp32 architecture of input file `/tmp/ccIFqSxU.o' is
> incompatible with aarch64 output; I believe this is not related to what
> I am testing)
>    c. Bootstrap with default works

Thanks for the comprehensive testing.  The linker error you see is 
because that the ILP32 support for aarch64*-*-linux* has not been added 
(Andrew Pinski has sent the patch series to enable the support here 
http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00282.html)

I also test the patch by building aarch64-none-elf cross compilers with 
binutils 2.23.2 and mainline, with default --with-multilibs-list.  It 
works well.

[snip]

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index b1b4eef..a53febc 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -5187,6 +5187,13 @@ aarch64_override_options (void)
>         aarch64_parse_tune ();
>       }
>
> +/* Issue error if assembler does not support -mabi and option ilp32
> +  is selected.  */

I'd prefer the comment to be "The compiler may have been configured with 
2.23.* binutils, which does not have support for ILP32."

> +#ifndef HAVE_AS_MABI_OPTION
> +  if (TARGET_ILP32)
> +    error ("Assembler does not supprt -mabi=ilp32");
> +#endif

supprt/support

> +
>     initialize_aarch64_code_model ();
>
>     aarch64_build_bitmask_table ();
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 91a22d5..a951b82 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -3495,6 +3495,35 @@ AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,
>   AC_MSG_RESULT($gcc_cv_lto_plugin)
>
>   case "$target" in
> +
> +  aarch64*-*-*)
> +    gcc_GAS_CHECK_FEATURE([-mabi option],
> +      gcc_cv_as_aarch64_mabi,,
> +      [-mabi=lp64], [.text],,,)
> +    if test x$gcc_cv_as_aarch64_mabi = xyes; then
> +	AC_DEFINE(HAVE_AS_MABI_OPTION, 1,
> +		  [Define if your assembler supports the -mabi option.])
> +    else
> +	if test x$with_abi = xilp32; then
> +	    AC_MSG_ERROR([Assembler does not support -mabi=ilp32.  Upgrade the Assembler.])
> +	fi
> +        if test x"$with_multilib_list" = xdefault; then
> +	    TM_MULTILIB_CONFIG=lp64
> +        else
> +	    aarch64_multilibs=`echo $with_multilib_list | sed -e 's/,/ /g'`
> +	    for aarch64_multilib in ${aarch64_multilibs}; do
> +		case ${aarch64_multilib} in
> +		    ilp32)
> +			AC_MSG_ERROR([Assembler does not support -mabi=ilp32.  Upgrade the Assembler.])
> +		    ;;
> +		    *)
> +		    ;;
> +		esac
> +	    done
> +	fi
> +    fi
> +    ;;
> +

I'm not very sure about the indent rules for configury files, but in 
other areas of configure.ac, it seems using a similar indent convention 
as in .c files.


Thanks,
Yufeng


>     # All TARGET_ABI_OSF targets.
>     alpha*-*-linux* | alpha*-*-*bsd*)
>       gcc_GAS_CHECK_FEATURE([explicit relocation support],
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index a8f9f8a..00c4f0d 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -3735,6 +3735,15 @@ removed and the system libunwind library will always be used.
>
>   @html
>   <hr />
> +@end html
> +@anchor{aarch64-x-x}
> +@heading aarch64*-*-*
> +Pre 2.24 binutils does not have support for selecting -mabi and does not
> +support ILP32.  If GCC 4.9 or later is built with pre 2.24, GCC will not
> +support option -mabi=ilp32.
> +
> +@html
> +<hr />
>   <!-- rs6000-ibm-aix*, powerpc-ibm-aix* -->
>   @end html
>   @anchor{x-ibm-aix}
>