From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::22c; helo=mail-it0-x22c.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22c.google.com (mail-it0-x22c.google.com [IPv6:2607:f8b0:4001:c0b::22c]) (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 D80A5221EA0C8 for ; Thu, 7 Dec 2017 11:47:57 -0800 (PST) Received: by mail-it0-x22c.google.com with SMTP id b5so17555489itc.3 for ; Thu, 07 Dec 2017 11:52:31 -0800 (PST) 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=b9+TJX7t2KFdXk8jqORC9tdru/yEnG2iIJRe5QK/xJo=; b=MKxVBzAyik0S/f+k7mdLFQQ5ig5ZhBaBOX7kOQSIkybnbamT3+lq2nuKmEAG+ucWI7 WzOBShNUwYPIxr+nPLOvshKwbyemRLYz7FaxMF6rl2QYtQdd/XSGW91RntyPTk43gyjH 2YEhe2s+piwLL4QbKfFfC5ZtFLeN1fIr0kPUw= 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=b9+TJX7t2KFdXk8jqORC9tdru/yEnG2iIJRe5QK/xJo=; b=WikRIkrSqxaZzWVXRcOXYf6TDfcGDT6doaN2FxEW3ISdVagtH9xT+nwMOp/dOJtzg6 xyYpua+dr25BFFLMgEKdDk8/H8zPQOABnOHVqivGKWbD/kDJ7+7eIwCQQY9g+uW2CREq kYdKfNWwR/Y/H40He3g9WzsmX9cKs6Nd0QT5IawFHN6Q/CujUxH1dqNiu3bnR+vRUEG1 Bp1aKrMP/u90nDPePIkO3n2JvJVW2CMkHaGbIqc3Nodur7ZFyHQOaEpFIf7X9a7dDkqG obvWn/BLo5YzN5Mln3e3U4E/jE+KKEiwgohrk2E2/LG8ULlsnVHZNfuQEKj7IADwiZYO YP2w== X-Gm-Message-State: AJaThX5gA66MiR3clTNkOuy3if8a46416aA5l7OsK3DHHAEPDiydCJqV s+45WERxdd69IVJUhLnd1aia8I2OyAJ7sD/HK8uG/Q== X-Google-Smtp-Source: AGs4zMZD4qjImYtCJXDGr2A9mf0tsuuyivvIy3b/5hEv/IhA3qcJvz5s0QOKoE8MqAtqh9c8xI/5vnepknPlUbRbUGY= X-Received: by 10.107.151.142 with SMTP id z136mr39903434iod.248.1512676350681; Thu, 07 Dec 2017 11:52:30 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Thu, 7 Dec 2017 11:52:30 -0800 (PST) In-Reply-To: References: <20171207151208.25648-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 7 Dec 2017 19:52:30 +0000 Message-ID: To: "Kinney, Michael D" Cc: Alexei Fedorov , "edk2-devel@lists.01.org" , "Gao, Liming" , Leif Lindholm 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 19:47:58 -0000 Content-Type: text/plain; charset="UTF-8" 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. > With LTO? > I will do some experiments on the currently supported > toolchains to see if the optimization is the same either > way. > Thank you. > I think the change you are suggesting is to improve > performance for optimization disabled builds by removing > an extra call. Is that correct? > 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. -- Ard. >> -----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. >> >