[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Added a few more minor comments.

It looks like the consensus on std-discussion is actually that your testcase has undefined behavior?  That seems like an awful conclusion, and the standard text doesn't really seem to support it. (I mean, I guess you could argue that " variable with thread storage duration shall be initialized before its first odr-use" means if it's not initialized, the behavior is undefined, but that's really confusing.)  But given that, I think we should submit a core issue, and hold off on merging this until we hear back from the committee.



================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:306
+  if (const auto *InitExpr = VD->getInit()) {
+    std::deque<const Stmt *> frontier;
+
----------------
If you don't care about the iteration order, using pop_back on a vector is faster.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:317
+          deps.insert(V);
+          auto V_Refs = enumerateVarInitDependencies(V);
+          deps.insert(V_Refs.begin(), V_Refs.end());
----------------
Do you need to recurse here?  It looks like the caller should handle that.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:469
+  for (const VarDecl *VD : UniqueVarsToInit)
+      OrderedVarInits.push_back(VD);
+
----------------
Indentation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66122/new/

https://reviews.llvm.org/D66122