Re: [PATCH v1 1/1] BaseTools: don't use WHOLEARCHIVE linker flag for VS2017 toolchain

[Roman Agafonov <Roman.Agafonov@aquantia.com>]

Hi Liming,

I don't have any custom options except a few ones to disable compiler warnings. AFAIK /GL is a compiler, not linker option, and it is used by EDK Visual Studio toolchains by default.

Of course, the results may vary from project to project. There difference may not be that big in all cases. Please try building some other package and compare the results. For example, this is what I get for OptionRomPkg compiled with two different toolchains. As you can see, the difference in size is abnormal.

pcfist@pcfist-pc:/mnt/c/src/UEFI/UDK2018/Build/OptionRomPkg$ ls -psh1 RELEASE_VS2017/X64 RELEASE_VS2015x86/X64
RELEASE_VS2015x86/X64:
total 212K
 20K AtapiPassThruDxe.efi
 24K AtapiPassThruDxe.rom
 28K Ax88772b.efi
 24K Ax88772.efi
 12K BltLibSample.efi
 24K CirrusLogic5430Dxe.efi
 12K CirrusLogic5430Dxe.rom
 24K FtdiUsbSerialDxe.efi
   0 MdePkg/
   0 OptionRomPkg/
8.0K TOOLS_DEF.X64
 36K UndiRuntimeDxe.efi

RELEASE_VS2017/X64:
total 424K
 56K AtapiPassThruDxe.efi
 56K AtapiPassThruDxe.rom
 60K Ax88772b.efi
 24K Ax88772.efi
 12K BltLibSample.efi
 56K CirrusLogic5430Dxe.efi
 28K CirrusLogic5430Dxe.rom
 56K FtdiUsbSerialDxe.efi
   0 MdePkg/
   0 OptionRomPkg/
8.0K TOOLS_DEF.X64
 68K UndiRuntimeDxe.efi

When comparing the .map files using difftool, I see a lot of unused functions from edk2 libraries (such as UefiDevicePathLib etc.) being included in the resulting executable, thus increasing its size. This is exactly what /WHOLEARCHIVE flag is intended for. My point here is whether this behavior is really desired, and given the fact that WHOLEARCHIVE had been removed from VS2015 toolchain previously, I don't see a reason to use it with VS2017 too.

Best regards,
Roman


From: Gao, Liming <liming.gao@intel.com>
Sent: Wednesday, June 5, 2019 6:47 PM
To: Roman Agafonov; devel@edk2.groups.io
Cc: Feng, Bob C; Zhu, Yonghong
Subject: RE: [PATCH v1 1/1] BaseTools: don't use WHOLEARCHIVE linker flag for VS2017 toolchain
 
I build edk2 OVMF with VS2015 and VS2017. Their image size are almost same. Have you the additional options to disable the optimization? In fact, /GL option will remove the unused function and logic.

> -----Original Message-----
> From: Roman Agafonov [mailto:Roman.Agafonov@aquantia.com]
> Sent: Wednesday, June 5, 2019 1:21 AM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: Re: [PATCH v1 1/1] BaseTools: don't use WHOLEARCHIVE linker flag for VS2017 toolchain
>
> Hi Liming,
>
> Sure. Here is what I get after building our NIC driver binary with VS2015x86 and VS2017 toolchains:
>
> pcfist@pcfist-pc:/mnt/c/src/uefi/udk2018$ du -h Build/xgbe_atl/RELEASE_VS2015x86/X64/xgbe_atl.efi
> Build/xgbe_atl/RELEASE_VS2017/X64/xgbe_atl.efi
> 36K     Build/xgbe_atl/RELEASE_VS2015x86/X64/xgbe_atl.efi
> 68K     Build/xgbe_atl/RELEASE_VS2017/X64/xgbe_atl.efi
>
> Best regards,
> Roman
>
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Tuesday, June 4, 2019 6:54 PM
> To: Roman Agafonov; devel@edk2.groups.io
> Cc: Feng, Bob C; Zhu, Yonghong
> Subject: RE: [PATCH v1 1/1] BaseTools: don't use WHOLEARCHIVE linker flag for VS2017 toolchain
>
> Can you show the size data with VS2017 and VS2015 for the same code?
>
> Thanks
> Liming
> > -----Original Message-----
> > From: Roman Agafonov [mailto:Roman.Agafonov@aquantia.com]
> > Sent: Tuesday, June 4, 2019 11:41 PM
> > To: devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> > Subject: [PATCH v1 1/1] BaseTools: don't use WHOLEARCHIVE linker flag for VS2017 toolchain
> >
> > I have noticed the resulting binaries are about twice as large when
> > using VS2017 toolchain compared to the ones built with VS2015. It appears
> > this is caused by /WHOLEARCHIVE linker flag used by this toolchain. This
> > flag was previously removed from VS2015 toolchain due to compatibility
> > issues. I believe it should not be used with VS2017 as well.
> >
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > Signed-off-by: Roman Agafonov <roman.agafonov@aquantia.com>
> > ---
> >  BaseTools/Conf/tools_def.template | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
> > index 26a2cf604f74..482a526f3052 100755
> > --- a/BaseTools/Conf/tools_def.template
> > +++ b/BaseTools/Conf/tools_def.template
> > @@ -1545,7 +1545,7 @@ NOOPT_VS2015x86_X64_DLINK_FLAGS    = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF
> >  *_VS2017_*_APP_FLAGS       = /nologo /E /TC
> >  *_VS2017_*_PP_FLAGS        = /nologo /E /TC /FIAutoGen.h
> >  *_VS2017_*_VFRPP_FLAGS     = /nologo /E /TC /DVFRCOMPILE /FI$(MODULE_NAME)StrDefs.h
> > -*_VS2017_*_DLINK2_FLAGS    = /WHOLEARCHIVE
> > +*_VS2017_*_DLINK2_FLAGS    =
> >  *_VS2017_*_ASM16_PATH      = DEF(VS2017_BIN_IA32)\ml.exe
> >
> >  ##################
> > --
> > 2.9.0.windows.1