<div class="gmail_quote">On Wed, Jun 6, 2012 at 2:17 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi all,<br>
<br>
Attached is the Clang-side patch for adding support for explicitly<br>
selected TLS models [1]. This adds support for the tls_model attribute<br>
[2].<br></blockquote><div><br></div><div>Very cool.</div><div><br></div><div>Should support for these be documented somewhere? It should at least go into the release notes for 3.2.</div><div><br></div><div>This parsing is duplicated a lot:</div>
<div><br></div><div><div>+    const TLSModelAttr *Attr = D.getAttr<TLSModelAttr>();</div><div>+    if (Attr->getModel() == "global-dynamic") {</div><div>+       TLM = llvm::GlobalVariable::GlobalDynamicTLSModel;</div>
<div>+    } else if (Attr->getModel() == "local-dynamic") {</div><div>+       TLM = llvm::GlobalVariable::LocalDynamicTLSModel;</div><div>+    } else if (Attr->getModel() == "initial-exec") {</div>
<div>+       TLM = llvm::GlobalVariable::InitialExecTLSModel;</div><div>+    } else if (Attr->getModel() == "local-exec") {</div><div>+       TLM = llvm::GlobalVariable::LocalExecTLSModel;</div><div>+    } else {</div>
<div>+      llvm_unreachable("unknown TLS model attribute");</div><div>+    }</div></div><div><br></div><div>Could you pull this into helper function that returns the llvm::GlobalVariable::ThreadLocalMode value? If you make it return not-thread-local in the event that nothing parses, you can use it in both the attribute checking code to diagnose invalid attributes, and in the rest of Clang to interpret them.</div>
<div><br></div><div><div>+    S.Diag(Attr.getLoc(), diag::err_attr_tlsmodel_arg)</div><div>+        << "\"global-dynamic\", \"local-dynamic\", "</div><div>+        "\"initial-exec\" or \"local-exec\"";</div>
</div><div><br></div><div>A big string like this should just go directly into the diagnostic text in the '.td' file.</div></div>