From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Permerror (SPF Permanent Error: More than 10 MX records returned) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 E5F10221EA0A5 for ; Thu, 7 Dec 2017 12:29:08 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Dec 2017 12:33:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,374,1508828400"; d="scan'208";a="11508369" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by fmsmga001.fm.intel.com with ESMTP; 07 Dec 2017 12:33:41 -0800 Received: from orsmsx116.amr.corp.intel.com (10.22.240.14) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 7 Dec 2017 12:33:41 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.187]) by ORSMSX116.amr.corp.intel.com ([10.22.240.14]) with mapi id 14.03.0319.002; Thu, 7 Dec 2017 12:33:40 -0800 From: "Kinney, Michael D" To: Ard Biesheuvel , "Kinney, Michael D" CC: Alexei Fedorov , "edk2-devel@lists.01.org" , Leif Lindholm , "Gao, Liming" Thread-Topic: [edk2] [PATCH] MdePkg/DebugLib; swap if conditions in ASSERT_[EFI|RETURN]_ERROR Thread-Index: AQHTb23ut8WaJ0TL/kqjkH352S+EyKM4hrqA//+TugCAAIkegIAAAQwA//9/6eCAAIiBgP//m0yAABEYHgAAD2zFMA== Date: Thu, 7 Dec 2017 20:33:39 +0000 Message-ID: References: <20171207151208.25648-1-ard.biesheuvel@linaro.org> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: [PATCH] MdePkg/DebugLib; swap if conditions in ASSERT_[EFI|RETURN]_ERROR 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: Thu, 07 Dec 2017 20:29:09 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] > On Behalf Of Ard Biesheuvel > Sent: Thursday, December 7, 2017 11:53 AM > To: Kinney, Michael D > Cc: Alexei Fedorov ; edk2- > devel@lists.01.org; Leif Lindholm > ; Gao, Liming > > Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > conditions in ASSERT_[EFI|RETURN]_ERROR >=20 > On 7 December 2017 at 19:49, Kinney, Michael D > wrote: > > Ard, > > > > I do not disagree with your logic. > > > > The current algorithm is based on data from a long > > time ago using what are now very old tool chains. > > >=20 > With LTO? Yes. The LTCG feature for VS tool chains. >=20 > > I will do some experiments on the currently supported > > toolchains to see if the optimization is the same > either > > way. > > >=20 > Thank you. >=20 > > I think the change you are suggesting is to improve > > performance for optimization disabled builds by > removing > > an extra call. Is that correct? > > >=20 > Well, for DEBUG builds, yes. But given that the function > call cannot > be optimized away (on non-LTO builds), it affects > optimized builds as > well. Do you mean compiler optimizations enabled, but linker optimizations disabled. >=20 > -- > Ard. >=20 >=20 > >> -----Original Message----- > >> From: Ard Biesheuvel > [mailto:ard.biesheuvel@linaro.org] > >> Sent: Thursday, December 7, 2017 9:43 AM > >> To: Kinney, Michael D > >> Cc: Alexei Fedorov ; edk2- > >> devel@lists.01.org; Gao, Liming > ; > >> Leif Lindholm > >> Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > >> conditions in ASSERT_[EFI|RETURN]_ERROR > >> > >> On 7 December 2017 at 17:36, Kinney, Michael D > >> wrote: > >> > Ard, > >> > > >> > With link time optimization, the current order > produces > >> > smaller code. > >> > > >> > >> I don't think it does. You are essentially saying that > >> DebugAssertEnabled() may resolve to a link time > constant > >> FALSE under > >> LTO. > >> > >> In that case, why would the following two statement > not > >> be equivalent? > >> > >> if (FALSE && EFI_ERROR (StatusParameter)) {} > >> > >> if (EFI_ERROR (StatusParameter) && FALSE) {} > >> > >> (which is essentially what a nested if () resolves to) > >> > >> In other words, the compiler is smart enough to drop > the > >> status check > >> in the second case, because it can see there are no > side > >> effects, and > >> the condition can never be made true anyway. > >> > >> > Without link time optimization, your patch will > produce > >> > smaller code, but not as small as link time > optimized > >> code. > >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel