From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from atlmailgw1.ami.com (atlmailgw1.ami.com [63.147.10.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9FE5721A18AAC for ; Mon, 27 Mar 2017 08:58:38 -0700 (PDT) X-AuditID: ac1060b2-a6fff70000000c96-0d-58d936aa7bb1 Received: from atlms2.us.megatrends.com (atlms2.us.megatrends.com [172.16.96.152]) (using TLS with cipher AES128-SHA (128/128 bits)) (Client did not present a certificate) by (Symantec Messaging Gateway) with SMTP id 8E.53.03222.AA639D85; Mon, 27 Mar 2017 11:58:37 -0400 (EDT) Received: from ATLMS1.us.megatrends.com ([fe80::8c55:daf0:ef05:5605]) by atlms2.us.megatrends.com ([fe80::29dc:a91e:ea0c:cdeb%12]) with mapi id 14.03.0123.003; Mon, 27 Mar 2017 11:58:34 -0400 From: Felix Poludov To: "Kinney, Michael D" , "Gao, Liming" , "edk2-devel@lists.01.org" Thread-Topic: [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined symbols, and MSFT/GCC tool chains. Thread-Index: AdKkmm2mcThMLZckRnawazzXsHkPjwCDpsPAABg95DAAATdlkAAApmqA Date: Mon, 27 Mar 2017 15:58:34 +0000 Message-ID: <9333E191E0D52B4999CE63A99BA663A002DEEF1308@atlms1.us.megatrends.com> References: <9333E191E0D52B4999CE63A99BA663A002DEEEDA95@atlms1.us.megatrends.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14D702B8A@shsmsx102.ccr.corp.intel.com> <9333E191E0D52B4999CE63A99BA663A002DEEF126B@atlms1.us.megatrends.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.16.99.93] content-transfer-encoding: quoted-printable MIME-Version: 1.0 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprEKsWRmVeSWpSXmKPExsWyRiBhhu5as5sRBq1neC32HDrKbLHi3gZ2 i46Of0wOzB6L97xk8uie/Y8lgCmqgdEmMS8vvySxJFUhJbU42VYpoCizLDG5UkkhM8VWyVBJ oSAnMTk1NzWvxFYpsaAgNS9FyY5LAQPYAJVl5imk5iXnp2TmpdsqeQb761pYmFrqGirZhWSk KmTmpeUX5SaWZObnKSTn55UAVaemAEUVEro4MyZcushecNCmYsFMrQbGW4ZdjJwcEgImEpsO X2bvYuTiEBKYzSRx4MULRgjnMKPErh3/WUCq2ARUJDadvcAMkhARmMgoMWHeR7YuRg4OYYF0 iVOXZEFqRAQyJF6eOscMYbtJtK9azAhiswioStye3cIEYvMKBErcnDOLGWLBWiaJD7e3sIEk OAVCJNr3LmcFsRkFxCS+n1oD1sAsIC5x68l8JohTBSSW7DnPDGGLSrx8/I8VwlaQ2PK+kx2i Xkdiwe5PbBC2tsSyha+ZIRYLSpyc+YRlAqPILCRjZyFpmYWkZRaSlgWMLKsYhRJLcnITM3PS yw31EnMz9ZLzczcxQpLAph2MLRfNDzEKcDAq8fCeUL0ZIcSaWFZcmXuIUYKDWUmE9xs3UIg3 JbGyKrUoP76oNCe1+BCjEzBcJjJLcYPiCBjp8cYGBlKiMI6hiZmJuZG5oaWJubGxkjhvCdP+ CCGBdGDiyU5NLUgtghnCxMEp1cAotu6W3Xs928A1J3/L333y+0B4ZEGc37wbL14xf1S/9SPO zEl807MJmSu2HdLKFC5wsrGasjFnu0uWiY4907Od93dE6fg+2rsqYhOj75Tcy8/EhdRm77+v NqHjesypx0sd7GcyHvn28VJ9lcuewiDpZceKjukc89135U1MIm/M4wcrIr0fhCrbKrEUZyQa ajEXFScCAIMoVQ8lAwAA Subject: Re: [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined symbols, and MSFT/GCC tool chains. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Mar 2017 15:58:38 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Mike, What do you think about defining GLOBAL_REMOVE_IF_UNREFERENCED in the tool c= hain definition file as an empty macro for a newer VS compilers? If this is done, as Liming pointed out, some code that compiles today may br= eak. If this is not done, variables declared with GLOBAL_REMOVE_IF_UNREFERENCED a= re not subject to the default policy of breaking the build if multiple defin= ed symbols are detected when MSFT tool chain is used. -----Original Message----- From: Kinney, Michael D [mailto:michael.d.kinney@intel.com] Sent: Monday, March 27, 2017 11:35 AM To: Felix Poludov; Gao, Liming; edk2-devel@lists.01.org; Kinney, Michael D Subject: RE: [RFC] GLOBAL_REMOVE_IF_UNREFERENCED, multiply defined symbols,= and MSFT/GCC tool chains. Felix, I prefer the default policy to break the build if multiple defined symbols a= re detected. Exceptions should only be allowed to support a specific compiler or a specif= ic 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 declarat= ions 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 new= er VS Compilers and preserve backwards compatibility with older VS compilers= . Thanks, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Felix Poludov > Sent: Monday, March 27, 2017 7:59 AM > To: Gao, Liming ; 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 imp= rovement. > 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.gao@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 p= latform 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 =3D_declspec(selectany) > > > > > >The advantages of these modifications are: > > > >- Better detection of on potential errors by breaking the build wh= en > >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/214a3b79417f64bf2faae74af42c > >1 > >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 th= en 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 the= n 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 proprietar= y to American Megatrends, Inc. This communication is intended to be read on= ly 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 not= ice that any distribution of this message, in any form, is strictly prohibit= ed. Please promptly notify the sender by reply e-mail or by telephone at 77= 0-246-8600, and then delete or destroy all copies of the transmission.