Ersek:
  I try O2 option. Compared to Ofast, there is a little different. I think it 
is acceptable. I will use O2 option. And, this warning message also exists 
without O2 enable. It is not introduced by this patch.


Tool                                               Compression time 
Decompression time

LzmaCompress (GCC O0)          3.476s           0.204s

LzmaCompress (GCC Ofast)     1.655s           0.107s

LzmaCompress (GCC O2)          1.687s          0.105s

Thanks
Liming
From: edk2-devel [mailto:[email protected]] On Behalf Of Gao, 
Liming
Sent: Friday, September 30, 2016 8:24 AM
To: Laszlo Ersek <[email protected]>
Cc: Justen, Jordan L <[email protected]>; [email protected]; Ard 
Biesheuvel <[email protected]>
Subject: Re: [edk2] [Patch 4/4] BaseTools Makefile: Enable Ofast option for GCC 
tool chain

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

Thanks
Liming
From: Laszlo Ersek [mailto:[email protected]]
Sent: Friday, September 30, 2016 2:30 AM
To: Gao, Liming
Cc: [email protected]<mailto:[email protected]>; Ard Biesheuvel ; 
Justen, Jordan L
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
_______________________________________________
edk2-devel mailing list
[email protected]<mailto:[email protected]>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to