[PATCH] D103988: SampleFDO] place the discriminator flag variable into the used list.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileLoaderBaseUtil.cpp:171
+  appendToUsed(*M, {new GlobalVariable(*M, Type::getInt1Ty(Context), true,
+                                       GlobalValue::WeakODRLinkage,
+                                       ConstantInt::getTrue(Context),
----------------
wenlei wrote:
> hoy wrote:
> > xur wrote:
> > > hoy wrote:
> > > > There's a `CommonLinkage` type and from its description, it looks like command variables should not be GC-ed but they can only have a zero value. Not sure if `-Wl,--export-dynamic-symbol` is needed with the common linkage type.
> > > > 
> > > > common
> > > > “common” linkage is most similar to “weak” linkage, but they are used for tentative definitions in C, such as “int X;” at global scope. Symbols with “common” linkage are merged in the same way as weak symbols, and they may not be deleted if unreferenced. common symbols may not have an explicit section, must have a zero initializer, and may not be marked ‘constant’. Functions and aliases may not have common linkage.
> > > Hongtao, thanks for pointing this out. I just tried and seemed "common" linkage worked -- I don't need to put the variable into used list and we don't need --export-dynamic-symbol to suppress the  GC. 
> > > 
> > > Using common linkage is a better solution here. I'll update the patch shortly.
> > LGTM, thanks for trying this out.
> Looks like similar things like `createIRLevelProfileFlagVar` and `createProfileFileNameVar` are using ExternalLinkage which would also suppress GC. (there's a check against supportsCOMDAT too). Is there a convention for these? 
"and they may not be deleted if unreferenced." This is the compiler behavior, not the linker behavior.

CommonLinkage is an externally visible definition not necessarily has duplicates (linkonce/linkonce_odr/available_externally) so the compiler cannot remove it. In the compiler, a CommonLinkage variable is considered interposable (so various IPO is suppressed). The linker can GC it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103988/new/

https://reviews.llvm.org/D103988