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. I will do some experiments on the currently supported toolchains to see if the optimization is the same either way. I think the change you are suggesting is to improve performance for optimization disabled builds by removing an extra call. Is that correct? Mike > -----Original Message----- > From: Ard Biesheuvel [mailto:[email protected]] > Sent: Thursday, December 7, 2017 9:43 AM > To: Kinney, Michael D <[email protected]> > Cc: Alexei Fedorov <[email protected]>; edk2- > [email protected]; Gao, Liming <[email protected]>; > Leif Lindholm <[email protected]> > Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > conditions in ASSERT_[EFI|RETURN]_ERROR > > On 7 December 2017 at 17:36, Kinney, Michael D > <[email protected]> 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

