From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22e.google.com (mail-it0-x22e.google.com [IPv6:2607:f8b0:4001:c0b::22e]) (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 4278521E977FA for ; Wed, 6 Sep 2017 10:09:38 -0700 (PDT) Received: by mail-it0-x22e.google.com with SMTP id f199so14276772ita.1 for ; Wed, 06 Sep 2017 10:12:28 -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=aWzzhrq7UzrrKnJlTzp97t6n+peuNSB21Ewhp01d/kA=; b=KjC6RRbnqhLqN76iMpIQAcJq7cm2eCcOCkuAhFCBb/R+TWuHqhPQocDCfvLPP5RZIt X7bghNvkaeQgcLuNcJCRh0L7SlOmbUboCxVnYAjuKxRW/qfmnOziyyE1VamS/I/tNpgc +EEzA4S5r7HoXK2Che9C8rMHJh4ycSMlpn/5s= 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=aWzzhrq7UzrrKnJlTzp97t6n+peuNSB21Ewhp01d/kA=; b=J2BbY6KSm3IxFgFmQ2w1XdXNfbkYi4LP81D6aZ33OqtE6RTa2FO2tuqi6FfsPeZE/s IecSmK6zZghU6QUOdbVvIrDyZd7fPQucL2FF19mxiRUUNfGCww5qxSbFn9WTJMsB0qow i11J8MZHSMDlu97nu5kcCtpaFx2qcZc+/NNjGex6helal3yrAe+k4wDo9PGZlA/jPU2H 6Yvwn1BQcxRhnGn0MgUynYv/mp/n2hh1uhSYG0wpno5T+BhsHTfCGQBtXgoRY5GB9v6j +DkKp5aROzUfggFF8HxJdh08lO2DLaTac+Ki8o8HewDRYLKWxLsnUcqBdh71t0qP8xk8 pn0Q== X-Gm-Message-State: AHPjjUh88u0z9yOxDmZ+5Sw05HgOSJfplAsSlylKwDlVODobGCx8OtqZ P3QFQOrmsgw+NQGfRCTtzPxQDOWB7oth X-Google-Smtp-Source: ADKCNb5MmzLDzHQ253tuEvNUmBPMrYjB1sX1Obex9j94kd9VEdj85BU6nKhR9N5RAz2PZDo11BtIyd/P4vv/8vYseF8= X-Received: by 10.36.150.198 with SMTP id z189mr486694itd.63.1504717947943; Wed, 06 Sep 2017 10:12:27 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Wed, 6 Sep 2017 10:12:27 -0700 (PDT) In-Reply-To: <0868f6e6-597c-6b1d-0864-7b6b1997adcf@redhat.com> References: <20170906164819.5082-1-lersek@redhat.com> <20170906164819.5082-2-lersek@redhat.com> <0868f6e6-597c-6b1d-0864-7b6b1997adcf@redhat.com> From: Ard Biesheuvel Date: Wed, 6 Sep 2017 18:12:27 +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 17:09:38 -0000 Content-Type: text/plain; charset="UTF-8" On 6 September 2017 at 18:09, Laszlo Ersek wrote: > On 09/06/17 18:56, Ard Biesheuvel wrote: >> 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 > > The option in question is "-Wno-unused-const-variable". But, that's a > side question. > No. The option is "-Wunused-const-variable", and adding the 'no-' prefix is the same as ignoring it. > If the above is acceptable to the MdePkg maintainers, it is certainly > good enough for me. In that case, I will reassign TianoCore BZ#700 to > MdePkg. (I was careful to write "desired handling of the new warning" in > the commit message, because I expected that the introduction of the GCC6 > toolchain just for this might not be universally liked.) > No. I think we should have fewer rather than more. I think the current collection in tools_def is ridiculous, but that is my personal opinion. > Personally I wouldn't like to submit the MdePkg patch myself, as GCC > pragmas are another delicacy that doesn't fit on my plate right now. (If > I happen to break something with it, I get to fix it too... Like I'm > fixing it now within OvmfPkg.) > > The point of this patch is to get OVMF back to a building state ASAP. > Once we fix BZ#700, I'm more than happy to revert the patch. I sought to > write the patch in a way that would be tolerable both short-term and > mid/long-term. > Fair enough.