From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 7A6DC1A1E30 for ; Wed, 5 Oct 2016 10:56:05 -0700 (PDT) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D69588E01F; Wed, 5 Oct 2016 17:56:04 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-104.phx2.redhat.com [10.3.116.104]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u95Hu3Lv024007; Wed, 5 Oct 2016 13:56:03 -0400 To: Ard Biesheuvel References: <1475631026-23928-1-git-send-email-yonghong.zhu@intel.com> <32c1e2a5-50e4-3152-0678-806f6bcc6bff@redhat.com> Cc: "edk2-devel@lists.01.org" , Liming Gao From: Laszlo Ersek Message-ID: <5bb7f8e0-4b08-21dd-d236-dca106fcd9ac@redhat.com> Date: Wed, 5 Oct 2016 19:56:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 05 Oct 2016 17:56:04 +0000 (UTC) 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 17:56:05 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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. Thanks Laszlo