From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x235.google.com (mail-it0-x235.google.com [IPv6:2607:f8b0:4001:c0b::235]) (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 ACFCD2095AE54 for ; Wed, 6 Sep 2017 09:53:45 -0700 (PDT) Received: by mail-it0-x235.google.com with SMTP id p6so11838376itb.1 for ; Wed, 06 Sep 2017 09:56:36 -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=7qqZ9LCAj670WrbxR+GYOboQXnRNUXQJLzox2kMBt6c=; b=FACcuitEHYzrVuc5nX0yWhE8GKAchvCMn1trTVkOtSejkCmAIusohSoZhXJ8OKZ7xt ZiZI+5CBQyS1t3zy0v28sb7qfH6RvBNA8SZHf1lbm/VoTgvHFrCNB/SC2P3XvdA4fk8O GHnqbgB3KN1Ui2x6VcoPOQlJ2n0Otoi8TlCwE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=7qqZ9LCAj670WrbxR+GYOboQXnRNUXQJLzox2kMBt6c=; b=KJ0QsqcuIZWxFmUdOs+ZDuCwIQG36+nG7It9YVCoud4EFHh21hLo/UdewDN9gwUdcC qxbV6cNqDkH9JZuzyjtT5zWDIihnbkRcuxKew4LaC7eqH/YcqgaiFvK2kleANBeRjXWU BDX5j7IC6x1sL9SRUIcaTFDL2wAzbCxWfphsVyKDIcLUN40oXuqwIgaDHNleSKos18DK G9uBCTEJW6u4BKqAU4P8Ft+qz3+FY3bpMDuWF3fxuKkpbYOATkym/Tc9DQ4yx3STdVyL 2L7h50ABr9BDE3PsfW+RY9GvwlBXv8a4sOx8iQoZprX5fhndtKZl5wZocvZXU/ZBAL3r /TUg== X-Gm-Message-State: AHPjjUiVK5LmBJopy94vAubYzz/biPNyt64/2jbdNe9nh4AeFvmGFI3D pP4x317+Z3xa3vw/wwjthgc6IBEw3C5/ X-Google-Smtp-Source: ADKCNb7HE1EidFZOG+tD7QnSrtZeT/HL5qOH8yDnvwNEPhJT0dfo33BBABgJPGjNeS1C2EVzxw0O2XdzkfJvj7w3/uk= X-Received: by 10.36.139.195 with SMTP id g186mr332800ite.128.1504716995377; Wed, 06 Sep 2017 09:56:35 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Wed, 6 Sep 2017 09:56:34 -0700 (PDT) In-Reply-To: <20170906164819.5082-2-lersek@redhat.com> References: <20170906164819.5082-1-lersek@redhat.com> <20170906164819.5082-2-lersek@redhat.com> From: Ard Biesheuvel Date: Wed, 6 Sep 2017 17:56:34 +0100 Message-ID: To: Laszlo Ersek Cc: edk2-devel-01 , Brijesh Singh , Jordan Justen , Thomas Lamprecht Subject: Re: [PATCH 1/1] OvmfPkg/IoMmuDxe: shut up "unused-const-variable" gcc-6 warning in RELEASE 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: Wed, 06 Sep 2017 16:53:46 -0000 Content-Type: text/plain; charset="UTF-8" On 6 September 2017 at 17:48, Laszlo Ersek wrote: > Starting with gcc-6, a new warning option called "-Wunused-const-variable" > has become available (and enabled by default under our build settings): > > https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html > > We should give it the same treatment as Ard gave "unused-but-set-variable" > in commit 20d00edf21d2 ("BaseTools/GCC: set -Wno-unused-but-set-variables > only on RELEASE builds", 2016-03-24); i.e., we should restrict the warning > to the DEBUG build target. > > However, because the new warning is gcc-6+ only, we cannot add > "-Wno-unused-const-variable" to any GCC5 macros in > "BaseTools/Conf/tools_def.template". While the GCC6 toolchain and/or the > desired handling of the new warning are investigated in > , suppress the warning > for gcc-6+ (in RELEASE builds) as follows: > > - Replace the "mBusMasterOperationName" array with the > BUS_MASTER_OPERATION_NAME(Expression, ShortName) macro that compares > Expression against EdkiiIoMmuOperationBusMaster, and in case > of a match, evaluates to "ShortName", stringified, > > - when composing the DEBUG message -- which the preprocessor might > eliminate from RELEASE builds, thereby removing its references to > variables --, build a ladder of comparisons with > BUS_MASTER_OPERATION_NAME(). > I wonder if we should put this in a header file somewhere. I really hate that we need to create new GCCx versions just for small deviations between versions of the compiler. I.e., add this to MdePkg/Include/Base.h #ifdef MDEPKG_NDEBUG // // Don't warn about unused but set variables: they may only be // referenced by DEBUG code. // #pragma GCC diagnostic ignored "-Wunused-but-set-variable" #if __GNUC__ >= 6 #pragma GCC diagnostic ignored "-Wunused-const-variable" #endif #endif > Cc: Ard Biesheuvel > Cc: Brijesh Singh > Cc: Jordan Justen > Cc: Thomas Lamprecht > Reported-by: Thomas Lamprecht > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > --- > OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 29 +++++++++++--------- > 1 file changed, 16 insertions(+), 13 deletions(-) > > diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c > index bc57de5b572b..dcd03d8f0474 100644 > --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c > +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c > @@ -45,17 +45,17 @@ STATIC LIST_ENTRY mRecycledMapInfos = INITIALIZE_LIST_HEAD_VARIABLE ( > #define COMMON_BUFFER_SIG SIGNATURE_64 ('C', 'M', 'N', 'B', 'U', 'F', 'F', 'R') > > // > -// ASCII names for EDKII_IOMMU_OPERATION constants, for debug logging. > +// The following macro builds the first two operands of a conditional (ternary) > +// operator that matches Expression against the EDKII_IOMMU_OPERATION constant > +// derived from ShortName. In case of a match, the replacement text evaluates > +// to an ASCII string literal that stands for the constant. The replacement > +// text stops before the colon (":"). The macro invocation site is supposed to > +// provide the colon and the third operand, either as another invocation of the > +// same macro, or as a default (fallback) string literal. > // > -STATIC CONST CHAR8 * CONST > -mBusMasterOperationName[EdkiiIoMmuOperationMaximum] = { > - "Read", > - "Write", > - "CommonBuffer", > - "Read64", > - "Write64", > - "CommonBuffer64" > -}; > +#define BUS_MASTER_OPERATION_NAME(Expression, ShortName) \ > + ((Expression) == (EdkiiIoMmuOperationBusMaster ## ShortName)) ? \ > + (# ShortName) > > // > // The following structure enables Map() and Unmap() to perform in-place > @@ -133,9 +133,12 @@ IoMmuMap ( > DEBUG_VERBOSE, > "%a: Operation=%a Host=0x%p Bytes=0x%Lx\n", > __FUNCTION__, > - ((Operation >= 0 && > - Operation < ARRAY_SIZE (mBusMasterOperationName)) ? > - mBusMasterOperationName[Operation] : > + (BUS_MASTER_OPERATION_NAME (Operation, Read) : > + BUS_MASTER_OPERATION_NAME (Operation, Write) : > + BUS_MASTER_OPERATION_NAME (Operation, CommonBuffer) : > + BUS_MASTER_OPERATION_NAME (Operation, Read64) : > + BUS_MASTER_OPERATION_NAME (Operation, Write64) : > + BUS_MASTER_OPERATION_NAME (Operation, CommonBuffer64) : > "Invalid"), > HostAddress, > (UINT64)((NumberOfBytes == NULL) ? 0 : *NumberOfBytes) > -- > 2.14.1.3.gb7cf6e02401b >