public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>, edk2-devel@lists.01.org
Cc: Yonghong Zhu <yonghong.zhu@intel.com>,
	Liming Gao <liming.gao@intel.com>,
	 Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH v2] BaseTools/GCC: set -Wno-unused-const-variables on RELEASE builds
Date: Fri, 8 Sep 2017 10:48:37 +0200	[thread overview]
Message-ID: <ae890fbc-7e88-6483-9066-0df95d1e6ac2@redhat.com> (raw)
In-Reply-To: <20170908070131.25158-1-t.lamprecht@proxmox.com>

On 09/08/17 09:01, Thomas Lamprecht wrote:
> TianoCore BZ#700 [1]
> 
> Set the '-Wno-unused-const-variables' in RELEASE builds with the
> GGC49 and GCC5 toolchain.
> 
> 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" [2]
> 
> 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 unknown '-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 4.9 [3] (and also earlier, for that
> matter), so add the flag to the GCC49 and GCC5 toolchain, even if
> both GCC versions do not supports it.
> GCC49 doesn't enables LTO whereas GCC5 does. As GCC 6.0 through 6.2
> had bugs relating to LTO there can be desire to use the GCC49 target
> even if compiling with GCC 6, see 432f1d83f77a.
> 
> 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://bugzilla.tianocore.org/show_bug.cgi?id=700
> [2] https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html#index-Wunused-const-variable
> [3] https://gcc.gnu.org/onlinedocs/gcc-4.9.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>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
> 
> changes v1 -> v2:
>  * add the flag also to the GCC49 toolchain to allow using GCC 6 with
>    LTO disabled builds, thanks for the hints Laszlo and Liming!
>  * adapt commit message slightly, especially to the fact that GCC49
>    gets the flag now too.
>  * CC Ard
> 
>  BaseTools/Conf/tools_def.template | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
> index ba1d1a16de..cbb5024c1b 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -5179,7 +5179,7 @@ RELEASE_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s
>  *_GCC49_IA32_NASM_FLAGS           = -f elf32
>  
>    DEBUG_GCC49_IA32_CC_FLAGS       = DEF(GCC49_IA32_CC_FLAGS) -Os
> -RELEASE_GCC49_IA32_CC_FLAGS       = DEF(GCC49_IA32_CC_FLAGS) -Os -Wno-unused-but-set-variable
> +RELEASE_GCC49_IA32_CC_FLAGS       = DEF(GCC49_IA32_CC_FLAGS) -Os -Wno-unused-but-set-variable -Wno-unused-const-variable
>    NOOPT_GCC49_IA32_CC_FLAGS       = DEF(GCC49_IA32_CC_FLAGS) -O0
>  
>  ##################
> @@ -5207,7 +5207,7 @@ RELEASE_GCC49_IA32_CC_FLAGS       = DEF(GCC49_IA32_CC_FLAGS) -Os -Wno-unused-but
>  *_GCC49_X64_NASM_FLAGS           = -f elf64
>  
>    DEBUG_GCC49_X64_CC_FLAGS       = DEF(GCC49_X64_CC_FLAGS) -Os
> -RELEASE_GCC49_X64_CC_FLAGS       = DEF(GCC49_X64_CC_FLAGS) -Os -Wno-unused-but-set-variable
> +RELEASE_GCC49_X64_CC_FLAGS       = DEF(GCC49_X64_CC_FLAGS) -Os -Wno-unused-but-set-variable -Wno-unused-const-variable
>    NOOPT_GCC49_X64_CC_FLAGS       = DEF(GCC49_X64_CC_FLAGS) -O0
>  
>  ##################
> @@ -5240,7 +5240,7 @@ RELEASE_GCC49_X64_CC_FLAGS       = DEF(GCC49_X64_CC_FLAGS) -Os -Wno-unused-but-s
>  *_GCC49_ARM_CC_XIPFLAGS          = DEF(GCC49_ARM_CC_XIPFLAGS)
>  
>    DEBUG_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -O0
> -RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-variable
> +RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable
>    NOOPT_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -O0
>  
>  ##################
> @@ -5272,7 +5272,7 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v
>    DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000
>    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
>  
> -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer
> +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny -fomit-frame-pointer
>  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)
>  
>    NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small
> @@ -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
> 


Reviewed-by: Laszlo Ersek <lersek@redhat.com>


  reply	other threads:[~2017-09-08  8:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-08  7:01 [PATCH v2] BaseTools/GCC: set -Wno-unused-const-variables on RELEASE builds Thomas Lamprecht
2017-09-08  8:48 ` Laszlo Ersek [this message]
2017-09-08  8:53   ` Ard Biesheuvel
2017-09-08  9:31 ` Gao, Liming
2017-09-08 19:05 ` Laszlo Ersek
2017-09-11  6:23   ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae890fbc-7e88-6483-9066-0df95d1e6ac2@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox