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::236; helo=mail-it0-x236.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x236.google.com (mail-it0-x236.google.com [IPv6:2607:f8b0:4001:c0b::236]) (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 3565F21B02820 for ; Thu, 7 Dec 2017 09:08:50 -0800 (PST) Received: by mail-it0-x236.google.com with SMTP id b5so16462397itc.3 for ; Thu, 07 Dec 2017 09:13:23 -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=SmQGRGa19TR5Uou+zIxh+VrD3SMiMUBmxnajGj3mers=; b=TsugInlZlec18/2mSTzjByO6u91A6ywjKnNejQmrdB9pMFVzzkXbUBbE/OSuhLaCxs +AMFUIevY0ZAAQDxZUAgDoNzND9Y7aWV7iQjRFIGXCbROZoOQwC5fHjl1XxIzCFyjmHi syCH3XZtH0vld9RtHc6zg3f27TothSU9TAYiQ= 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=SmQGRGa19TR5Uou+zIxh+VrD3SMiMUBmxnajGj3mers=; b=C9pFZEvYgz07j/pFcTEX07FBb40oWHUw3j9TzRjTCTUFpBD++v1+xlUfFy7rKpYRQJ ifCB2T+yiLHYCyuGoaeym6FBFc0ZMl/u0K5aIj00mT/PoydwmJ0N8Y6jBhjgajX2aQa9 bSk3/gMp41o9IYrT1QrgImqnxF7ktboI/JoLzoi0SD0iH250ebcbxhpRStUF0xCsjYqS F6cjvV4nhS0RqcN4slkHo3eWHvpTUcwZwx+1stae8U5nHH/G58354C/SmZpRQYC2Ol4z XN8WNQytGA1YHPGF2ddTnljoi55b4R4r8Ei992mXPpndt6cG6q3ee7ccNFzdFxujFITJ vohA== X-Gm-Message-State: AKGB3mKc79FlnhtOWsF+qCD4PAkKN2uZmOB4EH3Rq7DHO5K+TuuJU/qF bR512kCG3PsQUzNu9lPfXsNrtwV8HLFnehV11bIgdA== X-Google-Smtp-Source: AGs4zMaSJTUeNhWk3IXyQ7BV+NuN2lTJ05LsxKQ6oyd39h/lyxlAw7cJq7TYeks5UG6wtW/QV1frZ/CjhUbOr+MiCu4= X-Received: by 10.36.55.138 with SMTP id r132mr2209508itr.34.1512666802882; Thu, 07 Dec 2017 09:13:22 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Thu, 7 Dec 2017 09:13:22 -0800 (PST) In-Reply-To: References: <20171207151208.25648-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 7 Dec 2017 17:13:22 +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:08:50 -0000 Content-Type: text/plain; charset="UTF-8" On 7 December 2017 at 17:09, Ard Biesheuvel wrote: > 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. Please refer to these threads for details: https://lists.01.org/pipermail/edk2-devel/2017-December/018790.html https://lists.01.org/pipermail/edk2-devel/2017-December/018809.html