> -----Original Message----- > From: edk2-devel [mailto:[email protected]] > On Behalf Of Ard Biesheuvel > Sent: Thursday, December 7, 2017 11:53 AM > To: Kinney, Michael D <[email protected]> > Cc: Alexei Fedorov <[email protected]>; edk2- > [email protected]; Leif Lindholm > <[email protected]>; Gao, Liming > <[email protected]> > Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > conditions in ASSERT_[EFI|RETURN]_ERROR > > On 7 December 2017 at 19:49, Kinney, Michael D > <[email protected]> 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?
Yes. The LTCG feature for VS tool chains. > > > 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. Do you mean compiler optimizations enabled, but linker optimizations disabled. > > -- > Ard. > > > >> -----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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

