From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C41E51A1E3B for ; Wed, 5 Oct 2016 11:06:18 -0700 (PDT) Received: by mail-it0-x231.google.com with SMTP id o19so188397602ito.1 for ; Wed, 05 Oct 2016 11:06:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=CLPC816tQdrsQ59rTDkR/8OV71u4q6leoJJEXi0plaE=; b=GEYP5a/aKCADSN5IV3oHaZZA0RovFMzoyk3XkHBuf8EX9kHkUlpgHJzb+7KAKp7oxl +P5aRnLPZ70LQkdOaIWsd6H5/JdmGzArSaowUoqrXcvyh+7U8RqyPpX6bcyxWUyWzj43 Cc9XIBjvo+Gyd8vaygFajidhuk5aiBt+DVEjM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=CLPC816tQdrsQ59rTDkR/8OV71u4q6leoJJEXi0plaE=; b=BfjvgUseQUkJBNs44GIeVCnYaZsC3Zf1SLDEom3DtenTXiuEwOyGQB3z1TgYcCNSPJ xRwEsgbSpJADWYDoYWWmBO9lWmQECTqtSznomtYK6C3gOg++fnX8OAGqR+koaFbrhad9 BCOGku2vKnR9PAHnnTr0qDj5oeIdTr3LZm28sRPgMIDdEHmnLVxKw1RSYEgTcy+4WxFV h1UJeH8hZGjV3MIKs3GUGTwlcEe+sKFSbOXmmsodgXr7oGl3nA7s8+kSuPbzVIuQ7Hkv PUxQak/MvxZ7lCFGMfj3z9Hm1j05tTo9pez+StUsnf5XzzmOUwkF+/vCsd5MowA1hVVp xGCw== X-Gm-Message-State: AA6/9RnFwaTKF9otv5/APkTOEiH6rw8hdi2BHF6eqHc5g6QATHq7bctxTq1iuedfc7FxPicXTxnWjAcjqqHtrv81 X-Received: by 10.36.94.75 with SMTP id h72mr10667492itb.37.1475690776629; Wed, 05 Oct 2016 11:06:16 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.5.139 with HTTP; Wed, 5 Oct 2016 11:06:15 -0700 (PDT) In-Reply-To: <5bb7f8e0-4b08-21dd-d236-dca106fcd9ac@redhat.com> References: <1475631026-23928-1-git-send-email-yonghong.zhu@intel.com> <32c1e2a5-50e4-3152-0678-806f6bcc6bff@redhat.com> <5bb7f8e0-4b08-21dd-d236-dca106fcd9ac@redhat.com> From: Ard Biesheuvel Date: Wed, 5 Oct 2016 19:06:15 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Liming Gao Subject: Re: [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Oct 2016 18:06:19 -0000 Content-Type: text/plain; charset=UTF-8 On 5 October 2016 at 18:56, Laszlo Ersek wrote: > On 10/05/16 18:06, Ard Biesheuvel wrote: >> On 5 October 2016 at 15:48, Laszlo Ersek wrote: >>> On 10/05/16 03:30, Yonghong Zhu wrote: >>>> Update the tools_def.template to add NOOPT support with GCC tool chains. >>>> >>>> Cc: Liming Gao >>>> Cc: Laszlo Ersek >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Yonghong Zhu >>>> --- >>>> BaseTools/Conf/tools_def.template | 28 ++++++++++++++++++++++++++++ >>>> 1 file changed, 28 insertions(+) >>> >>> I thought I understood what was going on, but apparently I was wrong >>> about that. >>> >>> In this patch, we add or modify: >>> - NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay >>> - NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay >>> >>> So that part is fine with me. But then we also add / modify: >>> - NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS) >>> - NOOPT_GCC5_ARM_DLINK_FLAGS >>> >>> First I thought the latter set of changes was unnecessary, because "ld" >>> didn't use "-O". I checked the manual, and I was wrong: "ld" does know / >>> use "-O". So those changes are fine, I guess. >>> >> >> Yes, especially under LTO, in which case code generation is performed >> during the link stage, which should adhere to the same rules as the >> compiler. This not only applies to -O, but also to things like >> -march/-mcpu and -mstrict-align. This is why we pass all CFLAGS to the >> linker for the GCC5 LTO builds. >> >>> But then: is the patch *complete*? Because I can see some more DLINK >>> stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore >>> those? For example: >>> >>> *_GCC5_IA32_DLINK_FLAGS = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os >>> -Wl,-m,elf_i386,--oformat=elf32-i386 >>> >>> >>> *_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -Os >>> >>> Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include >>> -flto. (I don't know if "-flto" hampers source level debugging or not.) >>> >> >> The GCC man page documents -flto as being a bad idea, i.e., >> >> """ >> Link-time optimization does not work well with generation of debugging >> information. Combining -flto with -g is currently experimental and >> expected to produce unexpected results. >> """ >> >> (which raises a philosophical question as well, i.e., to which extent >> expected unexpected results are still unexpected results. But I >> digress ...) >> >> Another note: the DEBUG build for ARM and AARCH64 is essentially NOOPT >> already, not DEBUG. How does this patch intend to deal with that? > > It just copies the DEBUG settings to NOOPT (via deep copy, not by > reference). I believe that's OK. > OK, fair enough. Leif and I can look into this in the future.