[edk2,0/5] MdePkg BaseTools: GCC optimization for X64

On 07/14/16 15:16, Ard Biesheuvel wrote:
> This series is not an attempt to steal Steven's thunder. I was merely

> inspired by some of the changes he is proposing for GCC 5 and Clang

> support, which I noticed would be applicable to older versions of GCC

> as well.

> 

> The first patch fixes the issue that __builtin_unreachable() is not

> implemented by GCC 4.4 or earlier.

> 

> Patch #2 is Steven's patch to switch to the flavor of VA_LIST that is

> explicitly modeled after the MS implementation. This by itself is an

> improvement, since the open coded implementation that performs arithmetic

> on the address of explicit arguments to obtain the variadic arguments is

> fragile and difficult to maintain, and should be best avoided.

> 

> Patch #3 enabled -O2 optimization for X64/RELEASE all the way back to GCC44.

> I tested this change with both Ovmf and EmulatorPkg built in various ways

> and with various versions, with the caveat that I did not always use a matching

> binutils (i.e., of the same era). Since the issues this series deal with are

> all code generation issues, I think this is reasonable, but more testing would

> be appreciated.

> 

> Patch #4 applies the visibility 'hidden' GCC pragma globally. Please refer to

> the commit log for the motivation.

> 

> Patch #5 switches GCC/X64 to the PIC small code model, which results in smaller

> code.

> 

> Ard Biesheuvel (4):

>   MdePkg: avoid __builtin_unreachable() on GCC v4.4

>   BaseTools/tools_def: enable O2 optimization for GCC X64 builds

>   MdePkg X64: force 'hidden' visibility when building with -fpic

>   BaseTools/tools_def: switch GCC/X64 to the PIC small model

> 

> Shi, Steven (1):

>   MdePkg: Enable new MS VA intrinsics for GNUC x86 64bits build

> 

>  BaseTools/Conf/tools_def.template  | 18 +++++++-----

>  MdePkg/Include/Base.h              | 29 +++++++++++++++++++-

>  MdePkg/Include/X64/ProcessorBind.h | 10 +++++++

>  3 files changed, 49 insertions(+), 8 deletions(-)

>  mode change 100644 => 100755 MdePkg/Include/Base.h

> 

So, I did some tests.

First, the build tests. I built OVMF 84 times, with the following settings:

* Dimension 1: whether your and Steven's patches were applied or not.
The basis was always:
- upstream 04147690b59b,
- plus my personal patches that I constantly rebase,
- plus my pending "feat_ctrl_issue97_v3" patches,
- plus my work-in-progress build fixes for GCCxx and VS2015x86 warnings,
- plus OpenSSL 1.0.2g.

These patches don't interfere with your and Steven's work. So on top of
this basis, your and Steven's patches were either applied (rebased) or
not. This is a multiplier of 2.

* Dimension 2: the GCC toolchain used. Ranging from GCC44 through GCC49,
plus gcc-6.1 at the end (using GCC49 settings). Multiplier of 7.

* Dimension 3: DEBUG versus RELEASE build target. Factor of 2.

* Dimension 4: the OVMF platform. I used the following command lines:

- build -a IA32        -p OvmfPkg/OvmfPkgIa32.dsc    -D SMM_REQUIRE \
    -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE
- build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -D SMM_REQUIRE \
    -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE
- build         -a X64 -p OvmfPkg/OvmfPkgX64.dsc \
    -D SECURE_BOOT_ENABLE -D HTTP_BOOT_ENABLE

As you can see, Ia32 and Ia32X64 are built with the SMM driver stack;
X64 is built without it. (This is because edk2 S3 doesn't support
X64+SMM at the moment, so I either have to build X64 without SMM, or
disable S3 support on the QEMU command line. I find the latter more
inconvenient, plus Ia32X64 is just fine for 64-bit OS + SMM + S3.)
Anyhow, this is a factor of 3.

For these builds, I collected the log files and the report files. Values
in dimension 3 are not separated into separate files, because you can
pass -b DEBUG -b RELEASE at once, and the build utility will do the
right thing -- it will build both targets in succession. However, the
log file and the report file each will contain both DEBUG and RELEASE
output.

For the builds I also collected the OVMF_CODE.fd firmware binaries.

The good news is that all of the builds succeeded. You can draw size
comparisons from the build log files: see SECFV, PEIFV and DXEFV for
uncompressed sizes, and FVMAIN_COMPACT for the compressed size of
PEIFV+DXEFV together. (Thanks Mike for the reminder!)

I'll send you the link to the results archive off-list. (I tend not to
post stuff publicly that I cannot review in detail, purely out of
privacy concerns, and this archive is just too big for that. For example
I dislike disclosing full pathnames from my system: in commit messages I
always edit them out.)

Clearly if you distill any numbers from the archive, you are going to
review those in detail (and hopefully you won't disclose the entire
archive), so it should suffice to constrain my privacy concerns to you
personally. :)

-o-

Second, the run test. I executed two test scenarios here. For these, the
following dimensions were fixed:

- Dimension 1: your and Steven's patches were always applied.
- Dimension 2: the toolchain was always GCC48.
- Dimension 4: the OVMF platform was always X64.

* The first runtime test was repeating this test:

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/10963

Obviously, I modified this test a little bit: I removed the -Os / -O0
parts and differences, and simply built the UEFI binary with -b DEBUG
and -b RELEASE (--> Dimension 3 was variable). The bug that I had
originally reported in that thread did *not* reproduce this time. The -b
RELEASE binary worked fine.

* However, then I built my program -- find it attached -- with -b DEBUG
and -b RELEASE too, and the -b RELEASE binary is broken. Here's the
difference in output:

FS2:\> DebugEnrollDefaultKeys.efi
info: SetupMode=1 SecureBoot=0 SecureBootEnable=1 CustomMode=1 VendorKeys=0
info: SetupMode=0 SecureBoot=1 SecureBootEnable=1 CustomMode=0 VendorKeys=0
info: success

versus

FS2:\> OptEnrollDefaultKeys.efi
info: SetupMode=1 SecureBoot=0 SecureBootEnable=1 CustomMode=1 VendorKeys=0
error: EnrollListOfX509Certs("db",
D719B2CB-3D3A-4596-A3BC-DAD00E67656F): Invalid Parameter

I don't know why this happens, but it definitely relates to varargs --
the program uses them liberally.

NB: I did not test a RELEASE build of this program without your and
Steven's patches. (Remember that dimension 1 was fixed.)

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2,0/5] MdePkg BaseTools: GCC optimization for X64

Commit Message

Comments

Patch