[PATCH] D13515: Support for ThinLTO function importing and symbol linking.

[Tobias Edler von Koch via llvm-commits]

tobiasvk added inline comments.

================
Comment at: lib/Linker/LinkModules.cpp:676
@@ +675,3 @@
+  // can simply use the clone created in this module.
+  if (GVar && GVar->isConstant()) return false;
+  return true;
----------------
tejohnson wrote:
> tejohnson wrote:
> > davidxl wrote:
> > > tobiasvk wrote:
> > > > tejohnson wrote:
> > > > > davidxl wrote:
> > > > > > Unless the address taken bit is set (to handle the case address is used in comparison).
> > > > > Good catch. And unfortunately for GVars there is no address taken bit or easy check for this. For functions, there is a hasAddressTaken function that walks the uses, but not for GVars. But that isn't useful here in the importing context in any case, since we haven't even parsed all of the functions where it may be address taken. It will have to be recorded in the function index in the front end. For now I will simply promote all locals (and leave a comment about what would need to be done to reduce this scope).
> > > > Could you use the unnamed_addr bit?
> > > When promoting Constant Global, make sure the initializer is made available so that const prop can still happen.
> > Thanks for the tip!
> > 
> > I am not familiar with this attribute, but see it documented at http://llvm.org/docs/LangRef.html#global-variables:
> > 
> > "Global variables can be marked with unnamed_addr which indicates that the address is not significant, only the content. Constants marked like this can be merged with other constants if they have the same initializer. Note that a constant with significant address can be merged with a unnamed_addr constant, the result being a constant whose address is significant."
> > 
> > So it sounds like unnamed_addr variables are safe to be treated as non-address taken. I'm not sure under what context it is set though or if it covers all non-address taken const variables?
> > When promoting Constant Global, make sure the initializer is made available so that const prop can still happen.
> Right, the initializers are imported along with the variables.
> So it sounds like unnamed_addr variables are safe to be treated as non-address taken. I'm not sure under what context it is set though or if it covers all non-address taken const variables?

It's set by the GlobalOpt pass (mainly). GlobalOpt uses a helper, GlobalStatus, that does the analysis. I think you're safe to assume that it's non-address taken *if* the flag is set, but obviously GlobalOpt may not have been run.



http://reviews.llvm.org/D13515