From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxmox.maurer-it.com (proxmox.maurer-it.com [212.186.127.180]) (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 14FFF20958BD8 for ; Thu, 7 Sep 2017 23:23:29 -0700 (PDT) Received: from proxmox.maurer-it.com (localhost [127.0.0.1]) by proxmox.maurer-it.com (Proxmox) with ESMTP id 50C3B110E054; Fri, 8 Sep 2017 08:26:19 +0200 (CEST) To: Laszlo Ersek , "Gao, Liming" , "edk2-devel@lists.01.org" , Ard Biesheuvel References: <20170907145432.7571-1-t.lamprecht@proxmox.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14D78B7AF@shsmsx102.ccr.corp.intel.com> <6a362e1d-f29f-bced-6fec-93165ff93b44@redhat.com> From: Thomas Lamprecht Message-ID: <66625761-ba7a-e70e-cc72-368a639ac6e1@proxmox.com> Date: Fri, 8 Sep 2017 08:26:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <6a362e1d-f29f-bced-6fec-93165ff93b44@redhat.com> Subject: Re: [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds 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: Fri, 08 Sep 2017 06:23:29 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/07/2017 06:48 PM, Laszlo Ersek wrote: > On 09/07/17 17:04, Gao, Liming wrote: >> I suggest to also add this option in GCC49 tool chain RELEASE builds. GCC49 can be used for GCC 4.9 or the above version. It doesn't enable LTO. > > Good idea. There could be several reasons for picking GCC49; see e.g. > commit 432f1d83f77a ("OvmfPkg/build.sh: Use GCC49 toolchains with GCC > 6.[0-2]", 2016-12-06). > My rationale was to add it just to the "newest" ancestor of GCC 6 so that it could be even removed if in the future a GCC6 toolchain target still would get added, to reduce the "impact" of an unrecognized option warning on diagnostic output. But thanks to pointing out why it can make sense to use the GCC49 target even with GCC 6. > Thomas, can you please CC Ard on v2? Yes, will do so. cheers, Thomas > > Thank you! > Laszlo > > >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Thomas Lamprecht >>> Sent: Thursday, September 7, 2017 10:55 PM >>> To: edk2-devel@lists.01.org >>> Cc: Laszlo Ersek ; Gao, Liming >>> Subject: [edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds >>> >>> TianoCore BZ#700 >>> >>> This fixes the RELEASE build of OVMF with GCC in version 6 or newer. >>> GCC 6 added the '-Wunused-const-variable' warning, which gets >>> activated by '-Wunused-variable' and has the following behavior: >>> "Warn whenever a constant static variable is unused aside from its >>> declaration" [1] >>> >>> Commit 2ad6ba80a1bd58382bde6b994070f7c01d2fb48d introduced a case >>> where exactly this happens on a RELEASE build. All uses of the static >>> const variable are located in debug code only, which gets thrown out >>> by the compiler on RELEASE builds and thus triggers the >>> unused-const-variable warning. >>> >>> There is currently no GCC 6 toolchain target defined and doing so >>> would add a lot of boilerplate code. Instead, use the fact that GCC >>> ignores unkown '-Wno-*' options: >>> >>> "[...] if the -Wno- form is used [...] no diagnostic is produced for >>> -Wno-unknown-warning unless other diagnostics are being produced" >>> >>> This behavior is available in GCC 5 [2] (and also earlier, for that >>> matter), so add the flag to the GCC5 toolchain, even if GCC 5 itself >>> does not supports it. >>> >>> Orient the changes on 20d00edf21d2 which moved the >>> '-Wno-unused-but-set-variable' flag to RELEASE builds only, as there >>> it ensure that it does not gets raised if the only usage of a >>> variable is in (then collapsed) debug code. >>> >>> [1] https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html#index-Wunused-const-variable >>> [2] https://gcc.gnu.org/onlinedocs/gcc-5.4.0/gcc/Warning-Options.html >>> >>> Cc: Yonghong Zhu >>> Cc: Liming Gao >>> Cc: Laszlo Ersek >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Thomas Lamprecht >>> --- >>> >>> I hope I CCed the correct people and got the style right :) >>> >>> BaseTools/Conf/tools_def.template | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template >>> index ba1d1a16de..7db7a174c3 100755 >>> --- a/BaseTools/Conf/tools_def.template >>> +++ b/BaseTools/Conf/tools_def.template >>> @@ -5326,7 +5326,7 @@ RELEASE_GCC49_AARCH64_DLINK_FLAGS = DEF(GCC49_AARCH64_DLINK_FLAGS) >>> DEBUG_GCC5_IA32_CC_FLAGS = DEF(GCC5_IA32_CC_FLAGS) -flto -Os >>> DEBUG_GCC5_IA32_DLINK_FLAGS = DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os -Wl,-m,elf_i386,--oformat=elf32-i386 >>> >>> -RELEASE_GCC5_IA32_CC_FLAGS = DEF(GCC5_IA32_CC_FLAGS) -flto -Os -Wno-unused-but-set-variable >>> +RELEASE_GCC5_IA32_CC_FLAGS = DEF(GCC5_IA32_CC_FLAGS) -flto -Os -Wno-unused-but-set-variable >>> -Wno-unused-const-variable >>> RELEASE_GCC5_IA32_DLINK_FLAGS = DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os -Wl,-m,elf_i386,--oformat=elf32-i386 >>> >>> NOOPT_GCC5_IA32_CC_FLAGS = DEF(GCC5_IA32_CC_FLAGS) -O0 >>> @@ -5358,7 +5358,7 @@ RELEASE_GCC5_IA32_DLINK_FLAGS = DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os -Wl, >>> DEBUG_GCC5_X64_CC_FLAGS = DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO -Os >>> DEBUG_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os >>> >>> -RELEASE_GCC5_X64_CC_FLAGS = DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO -Os -Wno-unused-but-set-variable >>> +RELEASE_GCC5_X64_CC_FLAGS = DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO -Os -Wno-unused-but-set-variable >>> -Wno-unused-const-variable >>> RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os >>> >>> NOOPT_GCC5_X64_CC_FLAGS = DEF(GCC5_X64_CC_FLAGS) -O0 >>> @@ -5393,7 +5393,7 @@ RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os >>> DEBUG_GCC5_ARM_CC_FLAGS = DEF(GCC5_ARM_CC_FLAGS) -O0 >>> DEBUG_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) >>> >>> -RELEASE_GCC5_ARM_CC_FLAGS = DEF(GCC5_ARM_CC_FLAGS) -flto -Wno-unused-but-set-variable >>> +RELEASE_GCC5_ARM_CC_FLAGS = DEF(GCC5_ARM_CC_FLAGS) -flto -Wno-unused-but-set-variable >>> -Wno-unused-const-variable >>> RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto >>> -llto-arm -Wl,-plugin-opt=-pass-through=-llto-arm >>> >>> NOOPT_GCC5_ARM_CC_FLAGS = DEF(GCC5_ARM_CC_FLAGS) -O0 >>> @@ -5428,7 +5428,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS >>> DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 >>> DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 >>> >>> -RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -mcmodel=tiny >>> -fomit-frame-pointer >>> +RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable >>> -Wno-unused-const-variable -mcmodel=tiny -fomit-frame-pointer >>> RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto >>> -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch >>> >>> NOOPT_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small >>> -- >>> 2.11.0 >>> >>>