From patchwork Mon Dec 9 21:56:51 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yufeng Zhang X-Patchwork-Id: 22196 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ob0-f197.google.com (mail-ob0-f197.google.com [209.85.214.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 19A3B23908 for ; Mon, 9 Dec 2013 21:56:57 +0000 (UTC) Received: by mail-ob0-f197.google.com with SMTP id va2sf15867180obc.0 for ; Mon, 09 Dec 2013 13:56:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:date:from:user-agent :mime-version:to:cc:subject:references:in-reply-to:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=mJ930v34/g6ZoOdzqyIY6OQRgALLxeD5lYe/APXGwbk=; b=fNId3h4RR3dKadnc1uO4Z8PgO6JhGoiryW+psqZLwxsOzaEBWQO/WV73MMlgSECoz1 NEtAPtJ0K+bg7IL4a9TRBTHPE3Ea1K5LUhwfwyiP2jZ4tway/pXFT6e7Rte5phhSGOMS ib+rnp2BjRFg7F88rov4scq58n+74DD7mEJNWQSJEzE+3PVK60ULKWtsdRqLL/3MzIGw UWMeo9/sShFBr4adq6vgL+rahXwzqrnFuvvkufHngAqqYxodwemo1GeivYJkprTaU25H hmqsu6Q/z06LJd1baa6A/HUFH0kb6+3ZBq4TGf65Sp0i/+CARn6jSy1Qbd8KqCweRX3F JD7A== X-Gm-Message-State: ALoCoQk90HcOz2QFHSyAqR51n0QTnup87eyO0dn6+h4gSt2dIB/zKKEhG8Lv++Y00OwYfhpS1WcP X-Received: by 10.182.161.105 with SMTP id xr9mr7622892obb.31.1386626215996; Mon, 09 Dec 2013 13:56:55 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.81.133 with SMTP id a5ls2221675qey.21.gmail; Mon, 09 Dec 2013 13:56:55 -0800 (PST) X-Received: by 10.52.227.233 with SMTP id sd9mr220007vdc.53.1386626215863; Mon, 09 Dec 2013 13:56:55 -0800 (PST) Received: from mail-ve0-f179.google.com (mail-ve0-f179.google.com [209.85.128.179]) by mx.google.com with ESMTPS id ph5si4213527veb.52.2013.12.09.13.56.55 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 09 Dec 2013 13:56:55 -0800 (PST) Received-SPF: neutral (google.com: 209.85.128.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.179; Received: by mail-ve0-f179.google.com with SMTP id jw12so4235842veb.10 for ; Mon, 09 Dec 2013 13:56:55 -0800 (PST) X-Received: by 10.58.24.162 with SMTP id v2mr261727vef.39.1386626215588; Mon, 09 Dec 2013 13:56:55 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp139561vcz; Mon, 9 Dec 2013 13:56:55 -0800 (PST) X-Received: by 10.181.11.201 with SMTP id ek9mr16158675wid.54.1386626214363; Mon, 09 Dec 2013 13:56:54 -0800 (PST) Received: from service87.mimecast.com (service87.mimecast.com. [91.220.42.44]) by mx.google.com with ESMTP id w7si5586307wje.104.2013.12.09.13.56.53 for ; Mon, 09 Dec 2013 13:56:54 -0800 (PST) Received-SPF: pass (google.com: domain of yufeng.zhang@arm.com designates 91.220.42.44 as permitted sender) client-ip=91.220.42.44; Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Mon, 09 Dec 2013 21:56:53 +0000 Received: from [10.1.201.52] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 9 Dec 2013 21:56:51 +0000 Message-ID: <52A63CA3.9000304@arm.com> Date: Mon, 09 Dec 2013 21:56:51 +0000 From: Yufeng Zhang User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:8.0) Gecko/20111105 Thunderbird/8.0 MIME-Version: 1.0 To: Kugan CC: "gcc-patches@gcc.gnu.org" , "config-patches@gnu.org" , Marcus Shawcroft , "patches@linaro.org" Subject: Re: AARCH64 configure check for gas -mabi support References: <529F98A0.8020503@linaro.org> <52A1F8C0.70302@arm.com> <52A5A765.7000106@linaro.org> In-Reply-To: <52A5A765.7000106@linaro.org> X-OriginalArrivalTime: 09 Dec 2013 21:56:51.0704 (UTC) FILETIME=[9117BF80:01CEF529] X-MC-Unique: 113120921565300201 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: yufeng.zhang@arm.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.179 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , 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 > + * 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 >
> +@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 > +
> > @end html > @anchor{x-ibm-aix} > 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