[PATCH] D15300: Fix wrong external references to __emutls_v.*

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 9 12:54:59 PST 2015


rnk added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:394-395
@@ +393,4 @@
+  // __emutls_t.xyz is not in the symbol table.
+  if (TM.Options.EmulatedTLS && GV->getName().startswith("__emutls_v."))
+    return;
+
----------------
Remind me why we bother making an IR-level GlobalVariable if we never want to emit it?

================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3072
@@ -3071,3 +3071,3 @@
   std::string NameString = ("__emutls_v." + GA->getGlobal()->getName()).str();
   Module *VariableModule = const_cast<Module*>(GA->getGlobal()->getParent());
   StringRef EmuTlsVarName(NameString);
----------------
I guess I didn't pay enough attention on the first review. :(

We really shouldn't be modifying the IR in TargetLowering. We have some instances of that, but we shouldn't be adding more.

Long term, if we want to stick with the IR lowering strategy, maybe we should do it as an IR pass. I seem to recall Chandler objected strongly to that, though.

Anyway, setting the initializer and linkage here for now seems fine.

================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3088
@@ -3078,2 +3087,3 @@
+  }
   Entry.Node = DAG.getGlobalAddress(EmuTlsVar, dl, PtrVT);
   Entry.Ty = VoidPtrType;
----------------
If we only need the global variable to make this GlobalAddress node, there are other ways to do that, like DAG.getExternalSymbol and DAG.getMCSymbol.


http://reviews.llvm.org/D15300





More information about the llvm-commits mailing list