Re: [Patch 4/4] BaseTools Makefile: Enable Ofast option for GCC tool chain

["Gao, Liming" <liming.gao@intel.com>]

Laszlo:
   Thanks for your suggestion. I will try O2 option and see the performance and warning.

Thanks
Liming
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Friday, September 30, 2016 2:30 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: edk2-devel@ml01.01.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: Re: [edk2] [Patch 4/4] BaseTools Makefile: Enable Ofast option for GCC tool chain

Hi Liming,

On 09/29/16 16:12, Liming Gao wrote:
> Enable Ofast option to generate fast code for performance improvement.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao
> ---
> BaseTools/Source/C/Makefiles/header.makefile | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
> index f2041f8..ca2dc2e 100644
> --- a/BaseTools/Source/C/Makefiles/header.makefile
> +++ b/BaseTools/Source/C/Makefiles/header.makefile
> @@ -44,12 +44,12 @@ ARCH_INCLUDE = -I $(MAKEROOT)/Include/AArch64/
> endif
>
> INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKEROOT)/Include/ -I $(MAKEROOT)/Include/IndustryStandard -I $(MAKEROOT)/Common/ -I .. -I . $(ARCH_INCLUDE)
> -BUILD_CPPFLAGS = $(INCLUDE)
> +BUILD_CPPFLAGS = $(INCLUDE) -Ofast
> ifeq ($(DARWIN),Darwin)
> # assume clang or clang compatible flags on OS X
> -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -nostdlib -c -g
> +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -c -g
> else
> -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -nostdlib -c -g
> +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-unused-result -nostdlib -c -g
> endif
> BUILD_LFLAGS =
> BUILD_CXXFLAGS =
>

are you sure -Ofast is a good idea? The gcc manual says,

-Ofast
Disregard strict standards compliance. -Ofast enables all -O3
optimizations. It also enables optimizations that are not valid for
all standard-compliant programs. It turns on -ffast-math and the
Fortran-specific -fno-protect-parens and -fstack-arrays.

To me this sounds quite scary -- I'm worried it might break the base
utilities in various obscure ways, which in turn could break the
firmware built with these tools in obscure ways.

Most upstream projects and GNU/Linux distributions use -O2 for
performance-optimized builds. Can you try that please and see if the
performance improvements (relative to the current status) are still
acceptable? If so, I would strongly prefer -O2 over -Ofast.

Also, while building BaseTools with your series applied (using gcc-4.8),
I saw one warning issued:

> g++ -c -I Pccts/h -I .. -I ../Include/Common -I ../Include/ -I ../Include/IndustryStandard -I ../Common/ -I .. -I . -I ../Include/X64/ -Ofast VfrFormPkg.cpp -o VfrFormPkg.o
> VfrFormPkg.cpp: In member function 'void CIfrRecordInfoDB::IfrUpdateRecordInfoForDynamicOpcode(BOOLEAN)':
> VfrFormPkg.cpp:1360:91: warning: deprecated conversion from string constant to 'CHAR8* {aka char*}' [-Wwrite-strings]
> gCVfrErrorHandle.PrintMsg (0, "Error", "Can not find the adjust offset in the record.");
^
Thanks!
Laszlo