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::232; helo=mail-it0-x232.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x232.google.com (mail-it0-x232.google.com [IPv6:2607:f8b0:4001:c0b::232]) (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 50899221EA0B7 for ; Thu, 7 Dec 2017 09:05:05 -0800 (PST) Received: by mail-it0-x232.google.com with SMTP id t1so16398455ite.5 for ; Thu, 07 Dec 2017 09:09:38 -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=752aqCsrrmleUrkskZeSngRPLCshuGBcWtFVn+3m6wM=; b=OW9GzPOk+vOSdv22I+EuQuJ6WfY+AgU5qfDt/eapjD/uNGn24oTTWzzBgEdG7mbfNK vVGkPubELRUOrnZl8R0I8oXf57v6xsMauWmGfHvBq9fEeVYFO42tUXtTRnvtqD8B0Glk r63NWeZcr/eu4MJfNmgAzGkjBSQmcCIkkjpsI= 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=752aqCsrrmleUrkskZeSngRPLCshuGBcWtFVn+3m6wM=; b=N+k1iYIiQl/VGD9uFZ1C09oMSEFJtJGwm90tNWiJDsdGsM7OH0CTHMEMCOCoDneuTN 1Rx/N5wEXDb9JV2lVoBIUZ3ofYAESIUpJh2HD2fUfcPnuw+LJaPLb8b1RC8faa7rSDQU yOrtmbS54UQyUEIEfbQWLDoJarHoreQCa9puLZQlmSZbNj6jXco4MwrUOWrAep/5A5pY TP0b/uOb3Dzjg7c/kBZ66GQe5AzY5xkX48catWx84Hdzabk04SYnoweXYF9dJuXhLZ55 gWg/VpoNTniI0oscyGIVkaccKAOruCZsMVkkgP/fd0sAZfGDfSM+rFMai+sG10DMD4c6 ozpQ== X-Gm-Message-State: AKGB3mK8Yly8JI0t/5nnDpuxYZoWf/IPnVlEMic9oAI0tl4zKPthUjAf bd8A6Bejin50Pug9tMqFhDxbNdcqNaqbrikn1jx4MA== X-Google-Smtp-Source: AGs4zMYhCy3U+SGntEQb2zy72xCcmBOvPDrloKrqaRxyZYrtFbG7+KJ0OTwZ3ePh+/py1y8KgboyCj3gcnThtRQr3LQ= X-Received: by 10.36.71.83 with SMTP id t80mr1529334itb.48.1512666577836; Thu, 07 Dec 2017 09:09:37 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Thu, 7 Dec 2017 09:09:37 -0800 (PST) In-Reply-To: References: <20171207151208.25648-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 7 Dec 2017 17:09:37 +0000 Message-ID: To: "Kinney, Michael D" Cc: "edk2-devel@lists.01.org" , Leif Lindholm , "Gao, Liming" , Evan Lloyd , Alexei Fedorov 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 17:05:05 -0000 Content-Type: text/plain; charset="UTF-8" On 7 December 2017 at 17:01, Kinney, Michael D wrote: > Ard, > > The reason for the current ordering is for size optimization. > > The most common implementation of DebugAssertEnabled() uses > a FixedAtBuild PCD to determine if these are enabled. The > check of status can be optimized away if they are disabled. > If you reverse them, then the status check is always performed. > DebugAssertEnabled() is a function call that gets resolved at link time, and is not annotated as being free of side effects. So I agree that the implementation of that function could be optimized into a 'return true' or 'return false' depending on the compile time values of those PCDs, but the way the macro is defined currently, it still requires the function call to be made, and the conditional compare with a constant that follows will still be present in the code. What I am suggesting is to replace it with a comparison with a constant, and a conditional function call instead. This will not affect code size, but will only remove needless function calls at runtime.