| Message ID | 20171207151208.25648-1-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [edk2] MdePkg/DebugLib; swap if conditions in ASSERT_[EFI|RETURN]_ERROR | expand |
On 7 December 2017 at 15:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > ASSERT_EFI_ERROR () is currently defined as > > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) \ > do { \ > if (DebugAssertEnabled ()) { \ > if (EFI_ERROR (StatusParameter)) { \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter)); \ > _ASSERT (!EFI_ERROR (StatusParameter)); \ > } \ > } \ > } while (FALSE) > #else > #define ASSERT_EFI_ERROR(StatusParameter) > #endif > > This is suboptimal, given that the DebugAssertEnabled () call in the > outer if must be executed unconditionally, since the compiler does not > know that it does not have any side effects. Instead, let's swap the > two ifs, and only call DebugAssertEnabled () if StatusParameter contains > an error value to begin with. Do the same for ASSERT_RETURN_ERROR > as well. > I just noticed we could do the same for ASSERT () as well. > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Reported-by: Alexei Fedorov <Alexei.Fedorov@arm.com> > --- > MdePkg/Include/Library/DebugLib.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h > index 3a910e6a208b..8369c378e79c 100644 > --- a/MdePkg/Include/Library/DebugLib.h > +++ b/MdePkg/Include/Library/DebugLib.h > @@ -337,8 +337,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) \ > do { \ > - if (DebugAssertEnabled ()) { \ > - if (EFI_ERROR (StatusParameter)) { \ > + if (EFI_ERROR (StatusParameter)) { \ > + if (DebugAssertEnabled ()) { \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter)); \ > _ASSERT (!EFI_ERROR (StatusParameter)); \ > } \ > @@ -363,8 +363,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_RETURN_ERROR(StatusParameter) \ > do { \ > - if (DebugAssertEnabled ()) { \ > - if (RETURN_ERROR (StatusParameter)) { \ > + if (RETURN_ERROR (StatusParameter)) { \ > + if (DebugAssertEnabled ()) { \ > DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR (Status = %r)\n", \ > StatusParameter)); \ > _ASSERT (!RETURN_ERROR (StatusParameter)); \ > -- > 2.11.0 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. Mike > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Thursday, December 7, 2017 7:26 AM > To: edk2-devel@lists.01.org > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Gao, Liming > <liming.gao@intel.com>; Evan Lloyd <evan.lloyd@arm.com>; > Kinney, Michael D <michael.d.kinney@intel.com>; Alexei > Fedorov <Alexei.Fedorov@arm.com>; Ard Biesheuvel > <ard.biesheuvel@linaro.org> > Subject: Re: [PATCH] MdePkg/DebugLib; swap if conditions > in ASSERT_[EFI|RETURN]_ERROR > > On 7 December 2017 at 15:12, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > ASSERT_EFI_ERROR () is currently defined as > > > > #if !defined(MDEPKG_NDEBUG) > > #define ASSERT_EFI_ERROR(StatusParameter) > \ > > do { > \ > > if (DebugAssertEnabled ()) { > \ > > if (EFI_ERROR (StatusParameter)) { > \ > > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR > (Status = %r)\n", StatusParameter)); \ > > _ASSERT (!EFI_ERROR (StatusParameter)); > \ > > } > \ > > } > \ > > } while (FALSE) > > #else > > #define ASSERT_EFI_ERROR(StatusParameter) > > #endif > > > > This is suboptimal, given that the DebugAssertEnabled > () call in the > > outer if must be executed unconditionally, since the > compiler does not > > know that it does not have any side effects. Instead, > let's swap the > > two ifs, and only call DebugAssertEnabled () if > StatusParameter contains > > an error value to begin with. Do the same for > ASSERT_RETURN_ERROR > > as well. > > > > I just noticed we could do the same for ASSERT () as > well. > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel > <ard.biesheuvel@linaro.org> > > Reported-by: Alexei Fedorov <Alexei.Fedorov@arm.com> > > --- > > MdePkg/Include/Library/DebugLib.h | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/MdePkg/Include/Library/DebugLib.h > b/MdePkg/Include/Library/DebugLib.h > > index 3a910e6a208b..8369c378e79c 100644 > > --- a/MdePkg/Include/Library/DebugLib.h > > +++ b/MdePkg/Include/Library/DebugLib.h > > @@ -337,8 +337,8 @@ DebugPrintLevelEnabled ( > > #if !defined(MDEPKG_NDEBUG) > > #define ASSERT_EFI_ERROR(StatusParameter) > \ > > do { > \ > > - if (DebugAssertEnabled ()) { > \ > > - if (EFI_ERROR (StatusParameter)) { > \ > > + if (EFI_ERROR (StatusParameter)) { > \ > > + if (DebugAssertEnabled ()) { > \ > > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR > (Status = %r)\n", StatusParameter)); \ > > _ASSERT (!EFI_ERROR (StatusParameter)); > \ > > } > \ > > @@ -363,8 +363,8 @@ DebugPrintLevelEnabled ( > > #if !defined(MDEPKG_NDEBUG) > > #define ASSERT_RETURN_ERROR(StatusParameter) > \ > > do { > \ > > - if (DebugAssertEnabled ()) { > \ > > - if (RETURN_ERROR (StatusParameter)) { > \ > > + if (RETURN_ERROR (StatusParameter)) { > \ > > + if (DebugAssertEnabled ()) { > \ > > DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR > (Status = %r)\n", \ > > StatusParameter)); > \ > > _ASSERT (!RETURN_ERROR (StatusParameter)); > \ > > -- > > 2.11.0 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard, With link time optimization, the current order produces smaller code. Without link time optimization, your patch will produce smaller code, but not as small as link time optimized code. Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] > On Behalf Of Ard Biesheuvel > Sent: Thursday, December 7, 2017 9:13 AM > To: Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>; edk2- > devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; > Leif Lindholm <leif.lindholm@linaro.org> > Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > conditions in ASSERT_[EFI|RETURN]_ERROR > > 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 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 7 December 2017 at 17:36, Kinney, Michael D <michael.d.kinney@intel.com> 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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:ard.biesheuvel@linaro.org] > Sent: Thursday, December 7, 2017 9:43 AM > To: Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>; edk2- > devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; > Leif Lindholm <leif.lindholm@linaro.org> > Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > conditions in ASSERT_[EFI|RETURN]_ERROR > > On 7 December 2017 at 17:36, Kinney, Michael D > <michael.d.kinney@intel.com> 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 7 December 2017 at 19:49, Kinney, Michael D <michael.d.kinney@intel.com> 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? > 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. -- Ard. >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Thursday, December 7, 2017 9:43 AM >> To: Kinney, Michael D <michael.d.kinney@intel.com> >> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>; edk2- >> devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; >> Leif Lindholm <leif.lindholm@linaro.org> >> Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if >> conditions in ASSERT_[EFI|RETURN]_ERROR >> >> On 7 December 2017 at 17:36, Kinney, Michael D >> <michael.d.kinney@intel.com> 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] > On Behalf Of Ard Biesheuvel > Sent: Thursday, December 7, 2017 11:53 AM > To: Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>; edk2- > devel@lists.01.org; Leif Lindholm > <leif.lindholm@linaro.org>; Gao, Liming > <liming.gao@intel.com> > Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > conditions in ASSERT_[EFI|RETURN]_ERROR > > On 7 December 2017 at 19:49, Kinney, Michael D > <michael.d.kinney@intel.com> 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:ard.biesheuvel@linaro.org] > >> Sent: Thursday, December 7, 2017 9:43 AM > >> To: Kinney, Michael D <michael.d.kinney@intel.com> > >> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>; edk2- > >> devel@lists.01.org; Gao, Liming > <liming.gao@intel.com>; > >> Leif Lindholm <leif.lindholm@linaro.org> > >> Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if > >> conditions in ASSERT_[EFI|RETURN]_ERROR > >> > >> On 7 December 2017 at 17:36, Kinney, Michael D > >> <michael.d.kinney@intel.com> 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 > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 7 December 2017 at 20:33, Kinney, Michael D <michael.d.kinney@intel.com> wrote: >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] >> On Behalf Of Ard Biesheuvel >> Sent: Thursday, December 7, 2017 11:53 AM >> To: Kinney, Michael D <michael.d.kinney@intel.com> >> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>; edk2- >> devel@lists.01.org; Leif Lindholm >> <leif.lindholm@linaro.org>; Gao, Liming >> <liming.gao@intel.com> >> Subject: Re: [edk2] [PATCH] MdePkg/DebugLib; swap if >> conditions in ASSERT_[EFI|RETURN]_ERROR >> >> On 7 December 2017 at 19:49, Kinney, Michael D >> <michael.d.kinney@intel.com> 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. > Basically, yes. LTO has only been added recently for GCC5 on ARM/AARCH64, and we are currently adding support for CLANG38 as well. CLANG35 and RVCT do not support LTO. So non-LTO still needs to be supported as well, and in some debugging/tracing contexts, having lots of needless function calls is making our lives difficult. (Hence my additional comment regarding ASSERT (), although I suppose in some cases, calculating the result of the expression could be more costly than the actual function call) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h index 3a910e6a208b..8369c378e79c 100644 --- a/MdePkg/Include/Library/DebugLib.h +++ b/MdePkg/Include/Library/DebugLib.h @@ -337,8 +337,8 @@ DebugPrintLevelEnabled ( #if !defined(MDEPKG_NDEBUG) #define ASSERT_EFI_ERROR(StatusParameter) \ do { \ - if (DebugAssertEnabled ()) { \ - if (EFI_ERROR (StatusParameter)) { \ + if (EFI_ERROR (StatusParameter)) { \ + if (DebugAssertEnabled ()) { \ DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter)); \ _ASSERT (!EFI_ERROR (StatusParameter)); \ } \ @@ -363,8 +363,8 @@ DebugPrintLevelEnabled ( #if !defined(MDEPKG_NDEBUG) #define ASSERT_RETURN_ERROR(StatusParameter) \ do { \ - if (DebugAssertEnabled ()) { \ - if (RETURN_ERROR (StatusParameter)) { \ + if (RETURN_ERROR (StatusParameter)) { \ + if (DebugAssertEnabled ()) { \ DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR (Status = %r)\n", \ StatusParameter)); \ _ASSERT (!RETURN_ERROR (StatusParameter)); \
ASSERT_EFI_ERROR () is currently defined as #if !defined(MDEPKG_NDEBUG) #define ASSERT_EFI_ERROR(StatusParameter) \ do { \ if (DebugAssertEnabled ()) { \ if (EFI_ERROR (StatusParameter)) { \ DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter)); \ _ASSERT (!EFI_ERROR (StatusParameter)); \ } \ } \ } while (FALSE) #else #define ASSERT_EFI_ERROR(StatusParameter) #endif This is suboptimal, given that the DebugAssertEnabled () call in the outer if must be executed unconditionally, since the compiler does not know that it does not have any side effects. Instead, let's swap the two ifs, and only call DebugAssertEnabled () if StatusParameter contains an error value to begin with. Do the same for ASSERT_RETURN_ERROR as well. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reported-by: Alexei Fedorov <Alexei.Fedorov@arm.com> --- MdePkg/Include/Library/DebugLib.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel