This basically looks good to me, but we really need to get the Itanium ABI to 
cover this.

================
Comment at: lib/CodeGen/CGDecl.cpp:133-134
@@ -132,4 @@
-    // can be uniqued.
-    if (D.isExternallyVisible()) {
-      Linkage = llvm::GlobalValue::LinkOnceODRLinkage;
-
----------------
Do you have tests for this change? It seems to imply that dllimport, dllexport, 
and selectany now work on static local variables and didn't previously.

================
Comment at: lib/CodeGen/CGDecl.cpp:138-140
@@ -143,5 +137,5 @@
 
   if (D.hasExternalStorage())
     // Don't emit it now, allow it to be emitted lazily on its first use.
     return;
 
----------------
Hmm, I wonder why we don't do this for static locals too. Emitting it both 
eagerly and with a discardable linkage makes very little sense to me...

================
Comment at: lib/CodeGen/CodeGenModule.cpp:2873-2874
@@ -2862,5 +2872,4 @@
   // Create a global variable for this lifetime-extended temporary.
-  llvm::GlobalVariable *GV =
-    new llvm::GlobalVariable(getModule(), Type, Constant,
-                             llvm::GlobalValue::PrivateLinkage,
-                             InitialValue, Name.c_str());
+  llvm::GlobalValue::LinkageTypes Linkage =
+      GetLLVMLinkageVarDefinition(VD, Constant);
+  unsigned AddrSpace = GetGlobalVarAddressSpace(
----------------
What should the linkage of the temporary be if the linkage of the variable is 
strong? Seems we have two choices here:

  1) Always the same as that of the variable. This creates more symbols than 
are strictly necessary, but allows us to emit the variable as 
available_externally in some circumstances.
  2) If the variable is strong, the temporary is private.

The difference only seems to matter if it's possible to provide an 
available_externally definition (that is, for a static data member with an 
initializer specified within the class). Since our mangling doesn't collide 
with anyone else's, this will only matter if/when we start emitting the 
available_externally definitions.

http://reviews.llvm.org/D3515



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to