Re: [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined symbols, and MSFT/GCC tool chains.

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

Felix:
  This changes the default MSFT build behavior. It will impact all platforms even if this platform has no requirement to pass GCC build. I suggest to update platform DSC to enable it in MSFT tool chain if this platform needs to support MSFT and GCC both. 

In Base.h: I agree to define GLOBAL_REMOVE_IF_UNREFERENCED only when it is not defined. Then, Platform.dsc can append compiler option /D GLOBAL_REMOVE_IF_UNREFERE to the different value in [BuildOptions] section. 

#ifndef GLOBAL_REMOVE_IF_UNREFERENCED
  #define GLOBAL_REMOVE_IF_UNREFERENCED __declspec(selectany)
#endif

Thanks
Liming
>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Felix Poludov
>Sent: Friday, March 24, 2017 8:53 PM
>To: edk2-devel@lists.01.org
>Subject: [edk2] [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined
>symbols, and MSFT/GCC tool chains.
>
>Trying to add GCC support to projects based on MSFT tool chain, I'm keep
>stumbling into multiply defined symbol errors reported by GCC linker.
>An attempt to understand why the errors are not reported by the Microsoft
>linker lead me to GLOBAL_REMOVE_IF_UNREFERENCED macro.
>The purpose of the macro is to enable link time optimization of global
>variables.
>However, the way it's defined for MSFT tool chain (__declspec(selectany) )
>has a side effect of explicitly allowing multiple instances of a symbol defined
>with GLOBAL_REMOVE_IF_UNREFERENCED.
>For a while usage of the macro was the only option to enable global variable
>optimization.
>Starting from VS2013 compiler supports /Gw flag that enables global variable
>optimization without a special declarator.
>
>I propose to make the following modifications:
>
>1.      Change GLOBAL_REMOVE_IF_UNREFERENCED definition to an empty
>macro.
>
>Or more specifically, update macro definition in Base.h as follows:
>
>#ifndef GLOBAL_REMOVE_IF_UNREFERENCED
>
>#define GLOBAL_REMOVE_IF_UNREFERENCED
>
>#endif
>
>2.      Update VS2013 and VS2015 compiler flags to add /Gw option
>
>3.      Update compiler flags for older MSFT tool chains to define
>GLOBAL_REMOVE_IF_UNREFERENCED in a backward compatible manner for
>targets that enable optimization.
>
>/D GLOBAL_REMOVE_IF_UNREFERENCED =_declspec(selectany)
>
>
>The advantages of these modifications are:
>
>-        Better detection of on potential errors by breaking the build when
>symbol is defined more than once.
>
>-        Improved consistency between MSFT and GCC tool chains
>
>-        Improved link time optimization with VS2013 and newer MSFT tool chains.
>
>For example, mGaugeData in
>MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>is not declared as GLOBAL_REMOVE_IF_UNREFERENCED, so
>
>today performance library is linked with DXE Core even when performance
>measurements are disabled.
>
>The alternative option is to enable support of multiply defined symbols on
>GCC tool chain.
>One way to do it is by defining the macro as
>#define GLOBAL_REMOVE_IF_UNREFERENCED __attribute__((weak))
>
>However, I'm not sure that embracing multiple symbol definitions is a good
>idea.
>For example, see Ard's arguments in this commit comment
>https://github.com/tianocore/edk2/commit/214a3b79417f64bf2faae74af42c1
>b9d23f50dc8
>
>Thanks
>Felix
>
>Please consider the environment before printing this email.
>
>The information contained in this message may be confidential and
>proprietary to American Megatrends, Inc.  This communication is intended to
>be read only by the individual or entity to whom it is addressed or by their
>designee. If the reader of this message is not the intended recipient, you are
>on notice that any distribution of this message, in any form, is strictly
>prohibited.  Please promptly notify the sender by reply e-mail or by telephone
>at 770-246-8600, and then delete or destroy all copies of the transmission.
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel