From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BECAD2217CE28 for ; Wed, 6 Dec 2017 06:23:35 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7BB71C0587FC; Wed, 6 Dec 2017 14:28:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-175.rdu2.redhat.com [10.10.120.175]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8AA145F91F; Wed, 6 Dec 2017 14:28:04 +0000 (UTC) To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Leif Lindholm , "Gao, Liming" , "Zhu, Yonghong" , "Shi, Steven" , Evan Lloyd References: <20171206113917.6166-1-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: <5eb876bf-7362-0ab6-7931-76bd1b7ffd64@redhat.com> Date: Wed, 6 Dec 2017 15:28:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 06 Dec 2017 14:28:07 +0000 (UTC) Subject: Re: [PATCH] BaseTools/tools_def: add CLANG38 LTO versions for AARCH64 and ARM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Dec 2017 14:23:35 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/06/17 15:16, Ard Biesheuvel wrote: > On 6 December 2017 at 14:12, Laszlo Ersek wrote: >> On 12/06/17 12:39, Ard Biesheuvel wrote: >>> Extend the CLANG38 toolchain definition so it can be used for >>> ARM and AARCH64 as well. Note that this requires llvm-ar and >>> the LLVMgold.so linker plugin. >>> >>> In preparation of doing the same for GCC5, this toolchain version >>> also departs from the custom of using -O0 for DEBUG builds, which >>> makes them needlessly slow. Instead, let's add a NOOPT flavor as >>> well, and enable optimization for DEBUG like the other architectures >>> do. (Note that this will require some trivial changes to the platform >>> description files) >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Ard Biesheuvel >>> --- >>> BaseTools/Conf/tools_def.template | 98 +++++++++++++++++++- >>> 1 file changed, 95 insertions(+), 3 deletions(-) >>> >>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template >>> index 91b135c2e569..6ee720d7586e 100755 >>> --- a/BaseTools/Conf/tools_def.template >>> +++ b/BaseTools/Conf/tools_def.template >>> @@ -399,8 +399,8 @@ DEFINE DTC_BIN = ENV(DTC_PREFIX)dtc >>> # Intel(r) ACPI Compiler from >>> # https://acpica.org/downloads >>> # CLANG38 -Linux- Requires: >>> -# Clang v3.8, LLVMgold plugin and GNU binutils 2.26 targeting x86_64-linux-gnu >>> -# Clang v3.9 or later, LLVMgold plugin and GNU binutils 2.28 targeting x86_64-linux-gnu >>> +# Clang v3.8, LLVMgold plugin and GNU binutils 2.26 targeting x86_64-linux-gnu, aarch64-linux-gnu or arm-linux-gnueabi >>> +# Clang v3.9 or later, LLVMgold plugin and GNU binutils 2.28 targeting x86_64-linux-gnu, aarch64-linux-gnu or arm-linux-gnueabi >>> # Optional: >>> # Required to build platforms or ACPI tables: >>> # Intel(r) ACPI Compiler from >>> @@ -5652,6 +5652,7 @@ RELEASE_CLANG35_AARCH64_CC_FLAGS = DEF(CLANG35_AARCH64_CC_FLAGS) $(ARCHCC_FLAGS) >>> *_CLANG38_*_MAKE_PATH = make >>> *_CLANG38_*_*_DLL = ENV(CLANG38_DLL) >>> *_CLANG38_*_ASL_PATH = DEF(UNIX_IASL_BIN) >>> +*_CLANG38_*_DTC_PATH = DEF(DTC_BIN) >>> >>> *_CLANG38_*_APP_FLAGS = >>> *_CLANG38_*_ASL_FLAGS = DEF(IASL_FLAGS) >>> @@ -5663,7 +5664,8 @@ DEFINE CLANG38_X64_PREFIX = ENV(CLANG38_BIN) >>> DEFINE CLANG38_IA32_TARGET = -target i686-pc-linux-gnu >>> DEFINE CLANG38_X64_TARGET = -target x86_64-pc-linux-gnu >>> >>> -DEFINE CLANG38_ALL_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -Wno-empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-parentheses-equality -Wno-unknown-pragmas -Wno-tautological-constant-out-of-range-compare -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -msoft-float -mno-implicit-float -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-tautological-compare -Wno-unknown-warning-option -Wno-varargs >>> +DEFINE CLANG38_WARNING_OVERRIDES = -Wno-parentheses-equality -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Wno-empty-body -Wno-varargs >>> +DEFINE CLANG38_ALL_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) DEF(CLANG38_WARNING_OVERRIDES) -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -msoft-float -mno-implicit-float -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-unknown-warning-option >> >> At a quite superficial look, the patch seems OK to me. I'm just curious >> about one thing: what decides if a -Wno... option goes into >> CLANG38_WARNING_OVERRIDES? You left some -Wno... options out of it (kept >> them explicitly in the CLANG38_ALL_CC_FLAGS define). >> > > I split off some warning overrides that are shared between all > architectures, and left the x86 only ones in CLANG38_ALL_CC_FLAGS That's what I suspected; it just seemed strange that we wanted to suppress e.g. "-Waddress" for x86 only. ... I do see the point though -- suppressing warnings is always a messy business, so whenever we can opt for *enabling* warnings, we should. Not inheriting a -Wno... flag does just that. >> ... It seems like the CLANG38_ALL_CC_FLAGS is not used for the ARM / >> AARCH64 toolchains. "ALL" becomes sort of a misnomer then. Is that OK? >> > > Yes. In tools_def land, ALL still means 'both IA32 and X64', and I > have wasted too many cycles on trying to refactor the GCC toolchains > to care about it. Fair enough. Acked-by: Laszlo Ersek Thanks! Laszlo