config-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: AARCH64 configure check for gas -mabi support


From: Yufeng Zhang
Subject: Re: AARCH64 configure check for gas -mabi support
Date: Mon, 09 Dec 2013 21:56:51 +0000
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:8.0) Gecko/20111105 Thunderbird/8.0

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<address@hidden>
+       * 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:

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
     ;;


+
    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 />
address@hidden html
address@hidden
address@hidden 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.
+
address@hidden
+<hr />
  <!-- rs6000-ibm-aix*, powerpc-ibm-aix* -->
  @end html
  @anchor{x-ibm-aix}






reply via email to

[Prev in Thread] Current Thread [Next in Thread]