[PATCH] CodeGen: Emit globals with a constant initializer but no definition as available_externally.

[Benjamin Kramer]

================
Comment at: lib/CodeGen/CodeGenModule.cpp:1244-1247
@@ +1243,6 @@
+bool CodeGenModule::shouldEmitGlobalVariable(const VarDecl *VD) {
+  if (GetLLVMLinkageVarDefinition(VD, /*isConstant=*/true) !=
+      llvm::Function::AvailableExternallyLinkage)
+    return true;
+  return CodeGenOpts.OptimizationLevel != 0;
+}
----------------
Reid Kleckner wrote:
> This seems more intuitive as:
> 
>   if (isAvailableExternally)
>     return CGO.OptLevel != 0;
>   return true;
> 
> Also, how do we know isConstant is true?  Can you assert that it is like you do below?
Will fix the order. The isConstant flag has no effect for this path currently, but I can add the assert here too.

================
Comment at: lib/CodeGen/CodeGenModule.cpp:1846
@@ +1845,3 @@
+           "Variable with init but no definition should be constant!");
+    return llvm::GlobalValue::AvailableExternallyLinkage;
+  }
----------------
Reid Kleckner wrote:
> What if the variable is thread local, or fits any of the below special cases involving templates?  This probably goes towards the end of the if/else chain, not first, considering that it's for an optimization.
The problem is that we now allow certain types of globals to hit the method that didn't get here before, such as explicit template instantiations. For example std::string is explicitly instantiated in libc++, but std::string::npos is still a good fit for this optimization. GetGVALinkageForVariable asserts in that case, so the options are

- replicating the checks from GetGVALinkageForVariable here.
- leaving the check where it is. I'm not aware of any valid C++11 fragment that this code would fail for, but that's of course a weak argument when C++ is moving on :/

Any preferences?


http://llvm-reviews.chandlerc.com/D1982