[cfe-commits] [Patch] Support the tls_model attribute (PR9788)

[Chandler Carruth]

On Wed, Jun 6, 2012 at 2:17 AM, Hans Wennborg <hans at chromium.org> wrote:

> Hi all,
>
> Attached is the Clang-side patch for adding support for explicitly
> selected TLS models [1]. This adds support for the tls_model attribute
> [2].
>

Very cool.

Should support for these be documented somewhere? It should at least go
into the release notes for 3.2.

This parsing is duplicated a lot:

+    const TLSModelAttr *Attr = D.getAttr<TLSModelAttr>();
+    if (Attr->getModel() == "global-dynamic") {
+       TLM = llvm::GlobalVariable::GlobalDynamicTLSModel;
+    } else if (Attr->getModel() == "local-dynamic") {
+       TLM = llvm::GlobalVariable::LocalDynamicTLSModel;
+    } else if (Attr->getModel() == "initial-exec") {
+       TLM = llvm::GlobalVariable::InitialExecTLSModel;
+    } else if (Attr->getModel() == "local-exec") {
+       TLM = llvm::GlobalVariable::LocalExecTLSModel;
+    } else {
+      llvm_unreachable("unknown TLS model attribute");
+    }

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.

+    S.Diag(Attr.getLoc(), diag::err_attr_tlsmodel_arg)
+        << "\"global-dynamic\", \"local-dynamic\", "
+        "\"initial-exec\" or \"local-exec\"";

A big string like this should just go directly into the diagnostic text in
the '.td' file.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120606/4cc9237b/attachment.html>