[PATCH] D15300: New lower emutls pass, fix linkage bugs.

[Reid Kleckner via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


================
Comment at: lib/CodeGen/LowerEmuTLS.cpp:42
@@ +41,3 @@
+  bool addEmuTlsVar(Module &M, const GlobalVariable *GV);
+  static void copyLinkageVisibility(const GlobalVariable *from,
+                                    GlobalVariable *to) {
----------------
What about COMDAT TLS variables? Consider this code:
  template <typename T> struct A { static thread_local int x; };
  int getX() { return A<int>::x++; }

Multiple TUs may instantiate A<int>::x, and we need __emutls_v._ZN1AIiE1xE to go in a comdat section.

Perhaps we can handle this in a subsequent patch, but it's good to think about.


================
Comment at: lib/CodeGen/LowerEmuTLS.cpp:105-106
@@ +104,4 @@
+  // sizeof(word) should be the same as sizeof(void*) on target.
+  EVT PtrVT = MVT::getIntegerVT(DL.getPointerSizeInBits(0));
+  IntegerType *WordType = IntegerType::get(C, PtrVT.getSizeInBits());
+  PointerType *InitPtrType = InitValue ?
----------------
Don't create an EVT object in IR transforms, they aren't needed.

A better way to write this would be:
  IntegerType *WordType = DL.getIntPtrType(C, 0);

================
Comment at: lib/CodeGen/LowerEmuTLS.cpp:112-114
@@ +111,5 @@
+  StructType *EmuTlsVarType = StructType::create(ElementTypeArray);
+  EmuTlsVar = dyn_cast_or_null<GlobalVariable>(
+      M.getOrInsertGlobal(EmuTlsVarName, EmuTlsVarType));
+  assert(EmuTlsVar && "Failed to create emualted TLS control variable");
+  copyLinkageVisibility(GV, EmuTlsVar);
----------------
If you use cast<> instead of dyn_cast_or_null<>, this assertion will be performed for you implicitly.


http://reviews.llvm.org/D15300