> On Mar 27, 2017, at 8:35 AM, Kinney, Michael D <michael.d.kin...@intel.com> > wrote: > > Felix, > > I prefer the default policy to break the build if multiple defined symbols > are detected. >
+1 Thanks, Andrew Fish > Exceptions should only be allowed to support a specific compiler or a > specific level > of compiler optimizations. > > I do like the addition of the /Gw switch to the newer VS compilers. Adding > the current > GLOBAL_REMOVE_IF_UNREFERENCED macro to global variable declarations is a > manual process > that usually requires inspection of PE/COFF images to notice that data that > should have > been optimized away is still present. > > Adding the #ifndef also looks like a good way to adopt the /Gw switch in > newer VS > Compilers and preserve backwards compatibility with older VS compilers. > > Thanks, > > Mike > > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Felix >> Poludov >> Sent: Monday, March 27, 2017 7:59 AM >> To: Gao, Liming <liming....@intel.com>; edk2-devel@lists.01.org >> Subject: Re: [edk2] [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined >> symbols, and >> MSFT/GCC tool chains. >> >> Liming, >> >> Yes surrounding GLOBAL_REMOVE_IF_UNREFERENCED with #ifndef would be an >> improvement. >> Can you make this change? >> >> On the other note, don't you think that EDKII should have a generic policy >> regarding >> multiply defined symbols (whether they are allowed or not)? >> Today, they may or may not work depending on the compiler used. >> >> -----Original Message----- >> From: Gao, Liming [mailto:liming....@intel.com] >> Sent: Monday, March 27, 2017 12:49 AM >> To: Felix Poludov; edk2-devel@lists.01.org >> Cc: Gao, Liming >> Subject: RE: [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined symbols, >> and >> MSFT/GCC tool chains. >> >> 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-boun...@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 >> >> 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 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
