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

Reid Kleckner rnk at google.com
Thu Oct 24 10:37:04 PDT 2013



================
Comment at: lib/CodeGen/CodeGenModule.cpp:1846
@@ +1845,3 @@
+           "Variable with init but no definition should be constant!");
+    return llvm::GlobalValue::AvailableExternallyLinkage;
+  }
----------------
Benjamin Kramer wrote:
> 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?
I guess there's no GVALinkage value for available_externally and we probably don't want to add one, so it might make sense to me to keep this here.

Can you add a test case for something like std::string::npos where it should be considered available_externally?

I can also come up with weird test cases along the lines of:

  constexpr int foo() { return 1; }
  struct Holder {
    static const int thread_local f = foo();
  };
  const int *bar() {
    return &Holder::f;
  }

I'm not sure what the linkage of 'f' should actually be, but having this as a test case would be good.  :)



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



More information about the cfe-commits mailing list