Re: [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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 <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
>>> Subject: [edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds
>>>
>>> TianoCore BZ#700 <https://bugzilla.tianocore.org/show_bug.cgi?id=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 <yonghong.zhu@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>>> ---
>>>
>>> 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
>>>
>>>