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

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

Roman:

Below is my data. There is no big difference for their image size. My VS2017 version 15.9, VS2015 version 14.0. And, /GL is the global optimization compiler option. /LTCG is the linker option for the whole-program optimization. /WHOLEARCHIVE will force to link all obj files. It is helpful to detect the duplicated functions or the missing functions. But, /LTCG can do the global optimization to remove unused ones. Then, the final image should be the similar./WHOLEARCHIVE has some issue in VS2015. So, it is not enabled in VS2015. 

edk2 git version 20d2e5a125
edk2-platform git version 892766538f, now OptionRomPkg is in edk2-platforms

Directory of D:\AllPkg\Build\OptionRomPkg\RELEASE_VS2015x86\X64

06/06/2019  09:44 PM    <DIR>          .
06/06/2019  09:44 PM    <DIR>          ..
06/06/2019  09:44 PM            20,512 AtapiPassThruDxe.efi
06/06/2019  09:44 PM            20,992 AtapiPassThruDxe.rom
06/06/2019  09:44 PM            23,040 Ax88772.efi
06/06/2019  09:44 PM            26,464 Ax88772b.efi
06/06/2019  09:44 PM            10,208 BltLibSample.efi
06/06/2019  09:44 PM            21,376 CirrusLogic5430Dxe.efi
06/06/2019  09:44 PM            11,264 CirrusLogic5430Dxe.rom
06/06/2019  09:44 PM            21,664 FtdiUsbSerialDxe.efi
06/06/2019  09:44 PM    <DIR>          MdePkg
06/06/2019  09:44 PM    <DIR>          OptionRomPkg
06/06/2019  09:44 PM             6,685 TOOLS_DEF.X64
06/06/2019  09:44 PM            34,688 UndiRuntimeDxe.efi

Directory of D:\AllPkg\Build\OptionRomPkg\RELEASE_VS2017\X64

06/06/2019  09:41 PM    <DIR>          .
06/06/2019  09:41 PM    <DIR>          ..
06/06/2019  09:41 PM            20,864 AtapiPassThruDxe.efi
06/06/2019  09:41 PM            20,992 AtapiPassThruDxe.rom
06/06/2019  09:41 PM            23,360 Ax88772.efi
06/06/2019  09:41 PM            26,944 Ax88772b.efi
06/06/2019  09:41 PM            10,528 BltLibSample.efi
06/06/2019  09:41 PM            21,792 CirrusLogic5430Dxe.efi
06/06/2019  09:41 PM            11,264 CirrusLogic5430Dxe.rom
06/06/2019  09:41 PM            22,080 FtdiUsbSerialDxe.efi
06/06/2019  09:41 PM    <DIR>          MdePkg
06/06/2019  09:41 PM    <DIR>          OptionRomPkg
06/06/2019  09:41 PM             7,129 TOOLS_DEF.X64
06/06/2019  09:41 PM            35,104 UndiRuntimeDxe.efi

Thanks
Liming
> -----Original Message-----
> From: Roman Agafonov [mailto:Roman.Agafonov@aquantia.com]
> Sent: Thursday, June 6, 2019 8:20 PM
> 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,
> 
> 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