On 2/23/2019 1:16 AM, Laszlo Ersek wrote:
Yes, I was fully aware of that.
However:
The issue is that, in the BmReportLoadFailure() function, we do some
work*before* we call REPORT_STATUS_CODE_EX(). We have an ASSERT(), a
ZeroMem(), and a field assignment.
If status code reporting is disabled for EFI_ERROR_CODE in the platform,
then said work will be wasted. We can optimize this by checking for
ReportErrorCodeEnabled() up-front, because we know for sure that later
on we will report the status code with EFI_ERROR_CODE type.
In other words, this approach is similar to DEBUG_CODE(). In some cases,
logging a piece of information with DEBUG() takes non-trivial
computation. And it would be a waste, for example in RELEASE builds, to
perform the computation, and then throw away only the result (the log
message). Therefore the DEBUG_CODE macro is used, and the whole work is
eliminated in RELEASE builds.
The idea is the same here. If the compiler can statically deduce that
ReportErrorCodeEnabled() will always return FALSE -- for example because
the ReportStatusCodeLib instance in question looks at
"PcdReportStatusCodePropertyMask", and the PCD is Fixed-at-Build, and
the corresponding bit is clear --, then the compiler can eliminate the
entire BmReportLoadFailure() function. This is good for both flash usage
and for performance.
I'm fine either way, but first, please confirm again that you really
want me to remove the ReportErrorCodeEnabled() check, before pushing.
Thanks!
Laszlo
Thanks for the explanation. I am fine with both.
Reviewed-by: Ray Ni <ray...@intel.com>
--
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
