Re: [PATCH] MdePkg/DebugLib; swap if conditions in ASSERT_[EFI|RETURN]_ERROR

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 7 December 2017 at 17:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 7 December 2017 at 17:01, Kinney, Michael D
> <michael.d.kinney@intel.com> 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