Mike: I remember community suggests to use VS /Gw option to remove the global data, and then can define GLOBAL_REMOVE_IF_UNREFERENCED as empty or static.
Thanks Liming >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Kinney, Michael D >Sent: Friday, May 26, 2017 6:42 AM >To: af...@apple.com; Laszlo Ersek <ler...@redhat.com>; Kinney, Michael D ><michael.d.kin...@intel.com> >Cc: Wu, Hao A <hao.a...@intel.com>; edk2-devel@lists.01.org; Fan, Jeff ><jeff....@intel.com>; Felix Poludov <fel...@ami.com>; Ard Biesheuvel ><ard.biesheu...@linaro.org> >Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix >duplicate symbol > >Andrew, > >The VS compilers available when GLOBAL_REMOVE_IF_UNREFERENCED was >added referred to __declspec( selectany ) as putting the symbol into its own >comdat, so it was then available to be optimized away with the use of OPT:REF. > >I think it is time to re-evaluate the VS optimizers to see if they can optimize >away global variables without being decorated with__declspec( selectany ). If >we can remove __declspec( selectany ), then we have a path to use STATIC >properly to hide global variables that are not declared as extern in the >library >class. > >I will investigate some more. > >Mike > >From: af...@apple.com [mailto:af...@apple.com] >Sent: Thursday, May 25, 2017 2:26 PM >To: Laszlo Ersek <ler...@redhat.com> >Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Wu, Hao A ><hao.a...@intel.com>; edk2-devel@lists.01.org; Felix Poludov ><fel...@ami.com>; Kinney, Michael D <michael.d.kin...@intel.com>; Fan, >Jeff <jeff....@intel.com> >Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix >duplicate symbol > > >On May 25, 2017, at 2:02 PM, Laszlo Ersek ><ler...@redhat.com<mailto:ler...@redhat.com>> wrote: > >On 05/25/17 22:37, Andrew Fish wrote: > > > >On May 25, 2017, at 1:28 PM, Laszlo Ersek ><ler...@redhat.com<mailto:ler...@redhat.com>> wrote: > >On 05/25/17 22:11, Ard Biesheuvel wrote: > >On 25 May 2017 at 13:06, Kinney, Michael D ><michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> wrote: > >Laszlo and Andrew, > >With the information that has been collected on this thread, I >still think this patch in its original form is a good change >to resolve the this one specific duplicate symbol issue for all >tool chains. 'static' can not be mixed with >GLOBAL_REMOVE_IF_UNREFERENCED for MSFT tool chains, so renaming >the global variable is the easiest way to remove the duplicate. > >GLOBAL_REMOVE_IF_UNREFERENCED itself is problematic imo. I think it >was Felix who reported on this recently? > >STATIC is really the only sensible way to deal with this for symbols >that are only referenced by a single compilation unit. > > >I will continue to work on ways to detect duplicate symbols for >all tool chains and will enter a Bugzilla issue to for that >feature. > >In addition, the idea of detecting if a library is exporting more >than the library class defines is another good feature to consider >and I will enter a Bugzilla issue for that one as well. > >If we can find ways to both restrict the symbols exported by a >library and strip all symbols that are unused, then we can have >additional Bugzilla issues to perform that clean up on each >library instance that is exporting more than the library class. > >A static library is nothing more than an archive containing a >collection of object files. Sadly, that implies that we cannot >distinguish between symbols that may only be referenced by other >objects in the same static library and symbols that are exported to >the library client. > >Do we know for a fact that, with /OPT:REF, VS does not strip unused >*static* variables and functions? > >I mean, is it certain that *replacing* GLOBAL_REMOVE_IF_UNREFERENCED >with STATIC in this case would lead to a size increase? > >If that's the case, then I'm fine if we go ahead with this patch, I'd >just like to request that Mike please file some of those BZs, and please >reference them from the commit message (as the longer term solution), >before committing the patch. > >Clang will warn if you have unused static variables when warnings are cranked >up. > >~/work/Compiler>cat static.c >static unsigned char gTest[] = { 42 }; > >static int test () >{ > return 1; >} > >int main () >{ > return 0; >} >~/work/Compiler>clang -Os static.c -Wall >static.c:1:22: warning: unused variable 'gTest' [-Wunused-variable] >static unsigned char gTest[] = { 42 }; > ^ >static.c:3:12: warning: unused function 'test' [-Wunused-function] >static int test () > ^ >2 warnings generated. > >Sorry, my question was imprecise. > >Assume there is a public library function ("external linkage") that >calls a static function in the same library instance and uses a static >variable in the same library instance. Then this library instance is >linked into a driver, but the driver never actually calls the extern >function -- so the static variable and the static function too become >useless. > >In this case, will /OPT:REF remove the static variable and the static >function too? > >It seems counter-intuitive to me that an internal-only function or an >internal-only variable has to be declared extern (via >GLOBAL_REMOVE_IF_UNREFERENCED) just so it can be eliminated at link >time, if it is never referenced (transitively). > > >Laszlo, > >I agree. The LLVM LTO does not have an issue "doing the right thing". Seems >like static is also more of a compile time concept vs a link time (global >optimization) kind of thing? > >Given on VC++ GLOBAL_REMOVE_IF_UNREFERENCED maps to >__declspec( selectany ) I would guess maybe it has more to due with >supporting old non standard header files that can't change without breaking >compatibility. > >MSDN on __declspec( selectany ) : >A global data item can normally be initialized only once in an EXE or DLL >project. >selectany can be used in initializing global data defined by headers, when the >same header appears in more than one source file. selectany is available in >both the C and C++ compilers. > >Thanks, > >Andrew Fish > > > >Thanks >Laszlo >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto: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
