[PATCH] D18298: ThinLTO: do not promote GlobalVariable that have a specific section.

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Transforms/Utils/FunctionImportUtils.cpp:67
@@ -66,1 +66,3 @@
 
+  if (GVar && GVar->hasSection())
+    // Some sections like "__DATA,__cfstring" are "magic" and promotion is not
----------------
What happens when a reference to this GVar is imported to another module? Note that this change will prevent the renaming and promotion in the exporting module, but will still do so when importing the symbol (e.g. see getName() and other callsites below). 

I don't think that it can be addressed by simply preventing the renaming/promotion on the importing side as well. For one, isn't it possible that there will be name clashes between same-named local variables imported from different modules when they have assigned sections? Second, while perhaps unlikely, the local could presumably be eliminated in its original module if it is no longer needed there through some type of optimization, unless that same optimization happened on the imported copy.

(As an aside, I think there is a related issue with the preceding handling for local const variables due to the change not to import GVar defs - we expect to use the imported copy but won't have one, so the non-promoted, non-renamed imported ref won't be satisfied.)



http://reviews.llvm.org/D18298