diff mbox

AARCH64 configure check for gas -mabi support

Message ID 52A63CA3.9000304@arm.com
State Superseded
Headers show

Commit Message

Yufeng Zhang Dec. 9, 2013, 9:56 p.m. UTC 
Hi Kugan,

Thanks for the quick action.

On 12/09/13 11:20, Kugan wrote:
> Thanks Yufeng for the review.
>
> On 07/12/13 03:18, Yufeng Zhang wrote:
>
>>> >>  gcc trunk aarch64 bootstrapping fails with gas version 2.23.2 (with
>>> >>  error message similar to cannot compute suffix of object files) as this
>>> >>  particular version does not support -mabi=lp64. It succeeds with later
>>> >>  versions of gas that supports -mabi.
>> >
>> >  The -mabi option was introduced to gas when the support for ILP32 was
>> >  added.  Initially the options were named -milp32 and -mlp64:
>> >
>> >     http://sourceware.org/ml/binutils/2013-06/msg00178.html
>> >
>> >  and later on they were change to -mabi=ilp32 and -mabi=lp64 for
>> >  consistency with those in the aarch64 gcc:
>> >
>> >     http://sourceware.org/ml/binutils/2013-07/msg00180.html
>> >
>> >  The following gcc patch made the driver use the explicit option to drive
>> >  gas:
>> >
>> >     http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00083.html
>> >
>> >  It is a neglect of the backward compatibility with binutils 2.23.
>> >
>>> >>
>>> >>  Attached patch add checking for -mabi=lp64 and prompts upgradation. Is
>>> >>  this Ok?
>> >
>> >  I think instead of mandating the support for the -mabi option, the
>> >  compiler shall be changed able to work with binutils 2.23.  The 2.23
>> >  binutils have a good support for aarch64 and the main difference from
>> >  2.24 is the ILP32 support.  I think it is necessary to maintain the
>> >  backward compatibility, and it should be achieved by suppressing the
>> >  compiler's support for ILP32 when the -mabi option is not found
>> >  available in gas during the configuration time.
>> >
>> >  I had a quick look at areas need to be updated:
>> >
>> >  * multilib support
>> >
>> >  In gcc/config.gcc, the default and the only accepted value for
>> >  --with-multilib-list and --with-abi shall be lp64 when -mabi is not
>> >  available.
>> >
>> >  * -mabi option
>> >
>> >  I suggest we keep the -mabi option, but reject -mabi=ilp32 in
>> >  gcc/config/aarch64/aarch64.c:aarch64_override_options ()
>> >
>> >  * driver spec
>> >
>> >  In gcc/config/aarch64/aarch64-elf.h, the DRIVER_SELF_SPECS and ASM_SPEC
>> >  shall be updated to not pass/specify -mabi for gas.
>> >
>> >  * documentation
>> >
>> >  I think it needs to be mentioned in gcc/doc/install.texi the constraint
>> >  of using pre-2.24 binutils with aarch64 gcc that is 4.9 or later.
>> >
>> >  It is a quick scouting, but hopefully it has provided provide some
>> >  guidance.  If you need more help, just let me know.
>> >
>> >
>> >  Yufeng
>> >
>> >  P.s. some minor comments on the attached patch.
>> >
>>> >>
>>> >>  diff --git a/gcc/configure b/gcc/configure
>>> >>  index fdf0cd0..17b6e85 100755
>>> >>  --- a/gcc/configure
>>> >>  +++ b/gcc/configure
>> >
>> >  Diff result of auto-generation is usually excluded from a patch.
>> >
>>> >>  diff --git a/gcc/configure.ac b/gcc/configure.ac
>>> >>  index 91a22d5..730ada0 100644
>>> >>  --- a/gcc/configure.ac
>>> >>  +++ b/gcc/configure.ac
>>> >>  @@ -3532,6 +3532,15 @@ case "$target" in
>>> >>             [Define if your assembler supports the -no-mul-bug-abort
>>> >>  option.])])
>>> >>         ;;
>>> >>
>>> >>  + aarch64-*-*)
>> >
>> >  aarch64*-*-*
>> >
>>> >>  +    gcc_GAS_CHECK_FEATURE([-mabi option],
>>> >>  +      gcc_cv_as_aarch64_mabi,,
>>> >>  +      [-mabi=lp64], [.text],,,)
>>> >>  +    if test x$gcc_cv_as_aarch64_mabi = xno; then
>>> >>  +    AC_MSG_ERROR([Assembler support for -mabi=lp64 is required.
>>> >>  Upgrade the Assembler.])
>>> >>  +    fi
>>> >>  +    ;;
>>> >>  +
>>> >>       sparc*-*-*)
>>> >>         gcc_GAS_CHECK_FEATURE([.register], gcc_cv_as_sparc_register_op,,,
>>> >>           [.register %g2, #scratch],,
>>> >>
>> >
>> >
> Here is an attempt to do it the way you have suggested.
>
> 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
> +	Assembler does not support -mabi and option ilp32 is selected.

Assembler/assembler

> +	* doc/install.texi: Added note that building gcc 4.9 and after with pre
> +	2.24 binutils will not support -mabi=ilp32.
> +
>
>
>
> p.txt
>
>
> diff --git a/gcc/config/aarch64/aarch64-elf.h b/gcc/config/aarch64/aarch64-elf.h
> index 4757d22..b260b7c 100644
> --- a/gcc/config/aarch64/aarch64-elf.h
> +++ b/gcc/config/aarch64/aarch64-elf.h
> @@ -134,13 +134,19 @@
>     " %{!mbig-endian:%{!mlittle-endian:" ENDIAN_SPEC "}}" \
>     " %{!mabi=*:" ABI_SPEC "}"
>
> +#ifdef HAVE_AS_MABI_OPTION
> +#define ASM_MABI_SPEC	"%{mabi=*:-mabi=%*}"
> +#else
> +#define ASM_MABI_SPEC	"%{mabi=lp64*:}"

Is '*' necessary here?

> +#endif
> +
>   #ifndef ASM_SPEC
>   #define ASM_SPEC "\
>   %{mbig-endian:-EB} \
>   %{mlittle-endian:-EL} \
>   %{mcpu=*:-mcpu=%*} \
> -%{march=*:-march=%*} \
> -%{mabi=*:-mabi=%*}"
> +%{march=*:-march=%*}" \
> +ASM_MABI_SPEC
>   #endif
>
>   #undef TYPE_OPERAND_FMT
> 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.

>
>     initialize_aarch64_code_model ();
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 91a22d5..fcfdbdb 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -3532,6 +3532,19 @@ case "$target" in
>   		[Define if your assembler supports the -no-mul-bug-abort option.])])
>       ;;
>
> + aarch64*-*-*)

Alphabetically, this should be placed before alpha*.

> +    gcc_GAS_CHECK_FEATURE([-mabi option],
> +      gcc_cv_as_aarch64_mabi,,
> +      [-mabi=lp64], [.text],,,)
> +    if test $gcc_cv_as_aarch64_mabi = yes ; 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.])
> +    fi
> +    ;;

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:

+	fi
      fi
      ;;


> +
>     sparc*-*-*)
>       gcc_GAS_CHECK_FEATURE([.register], gcc_cv_as_sparc_register_op,,,
>         [.register %g2, #scratch],,
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index a8f9f8a..e4de2d2 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-*-*

aarch64*-*-*


Please also indicate what tests you have done with the patch.


Thanks,
Yufeng

> +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}
>
diff mbox

Patch

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