Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][driver] Add -mtls-dialect option #79031

Closed

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Jan 22, 2024

GCC an other ELF compilers support -mlts-dialect= options to control the
use of ELF TLS Descriptors as part of the TLS model. This patch only
adds support to the clang driver for the flag. Support for the
EnableTLSDesc code generation option in LLVM is added separately.

Verified

This commit was signed with the committer’s verified signature.
crazy-max CrazyMax
Created using spr 1.3.4
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Jan 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 22, 2024

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Paul Kirth (ilovepi)

Changes

GCC an other ELF compilers support -mlts-dialect= options to control the
use of ELF TLS Descriptors as part of the TLS model. This patch only
adds support to the clang driver for the flag. Support for the
EnableTLSDesc code generation option in LLVM is added separately.


Full diff: https://github.com/llvm/llvm-project/pull/79031.diff

6 Files Affected:

  • (modified) clang/include/clang/Basic/CodeGenOptions.def (+3)
  • (modified) clang/include/clang/Basic/CodeGenOptions.h (+5)
  • (modified) clang/include/clang/Driver/Options.td (+7)
  • (modified) clang/lib/CodeGen/BackendUtil.cpp (+2)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+8)
  • (added) clang/test/Driver/tls-dialect.c (+15)
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 2c4fb6745bc172f..6b96764b215be24 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -359,6 +359,9 @@ ENUM_CODEGENOPT(VecLib, llvm::driver::VectorLibrary, 3, llvm::driver::VectorLibr
 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)
 
+/// The default TLS model to use.
+ENUM_CODEGENOPT(DefaultTLSDialect, TLSDialect, 2, TraditionalTLSDialect)
+
 /// Bit size of immediate TLS offsets (0 == use the default).
 VALUE_CODEGENOPT(TLSSize, 8, 0)
 
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 6952b48e898a819..23180ababa956f1 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -72,6 +72,11 @@ class CodeGenOptions : public CodeGenOptionsBase {
     LocalExecTLSModel
   };
 
+  enum TLSDialect {
+    TraditionalTLSDialect,
+    TLSDescTLSDialect,
+  };
+
   enum StructReturnConventionKind {
     SRCK_Default,  // No special option was passed.
     SRCK_OnStack,  // Small structs on the stack (-fpcc-struct-return).
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d2e6c3ff721c27e..e0923dc2b5469bf 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4403,6 +4403,13 @@ def mtls_size_EQ : Joined<["-"], "mtls-size=">, Group<m_Group>,
   HelpText<"Specify bit size of immediate TLS offsets (AArch64 ELF only): "
            "12 (for 4KB) | 24 (for 16MB, default) | 32 (for 4GB) | 48 (for 256TB, needs -mcmodel=large)">,
   MarshallingInfoInt<CodeGenOpts<"TLSSize">>;
+def mtls_dialect_EQ : Joined<["-"], "mtls-dialect=">, Group<m_Group>,
+  Visibility<[ClangOption, CC1Option]>,
+  HelpText<"Use the given thread-local storage dialect">,
+  Values<"trad,desc">,
+  NormalizedValuesScope<"CodeGenOptions">,
+  NormalizedValues<["TraditionalTLSDialect", "TLSDescTLSDialect"]>,
+  MarshallingInfoEnum<CodeGenOpts<"DefaultTLSDialect">, "TraditionalTLSDialect">;
 def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group<m_Group>;
 def mdefault_build_attributes : Joined<["-"], "mdefault-build-attributes">, Group<m_Group>;
 def mno_default_build_attributes : Joined<["-"], "mno-default-build-attributes">, Group<m_Group>;
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index a6142d99f3b688d..0ab8156ab1f949b 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -401,6 +401,8 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.UniqueBasicBlockSectionNames =
       CodeGenOpts.UniqueBasicBlockSectionNames;
   Options.TLSSize = CodeGenOpts.TLSSize;
+  // TODO: Add correct codegen options in LLVM
+  // Options.TLSDesc = CodeGenOpts.getDefaultTLSDialect();
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index fead2e884030e21..232b7c48d56b296 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5812,6 +5812,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     Args.AddLastArg(CmdArgs, options::OPT_mtls_size_EQ);
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mtls_dialect_EQ)) {
+    // mlts-dialect= is ELF only
+    if (!Triple.isOSBinFormatELF())
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getOption().getName() << TripleStr;
+    Args.AddLastArg(CmdArgs, options::OPT_mtls_dialect_EQ);
+  }
+
   // Add the target cpu
   std::string CPU = getCPUName(D, Args, Triple, /*FromAs*/ false);
   if (!CPU.empty()) {
diff --git a/clang/test/Driver/tls-dialect.c b/clang/test/Driver/tls-dialect.c
new file mode 100644
index 000000000000000..42bf9ef42b0aa68
--- /dev/null
+++ b/clang/test/Driver/tls-dialect.c
@@ -0,0 +1,15 @@
+/// Options for ELF
+// RUN: %clang -### -target aarch64-linux-gnu -mtls-dialect=trad %s 2>&1 | FileCheck -check-prefix=TRAD %s
+// RUN: %clang -### -target aarch64-linux-gnu -mtls-dialect=desc %s 2>&1 | FileCheck -check-prefix=DESC %s
+
+/// Unsupported target
+// RUN: not %clang -target aarch64-unknown-windows-msvc -mtls-dialect=trad %s 2>&1 | FileCheck -check-prefix=UNSUPPORTED-TARGET %s
+// RUN: not %clang -target aarch64-unknown-windows-msvc -mtls-dialect=desc %s 2>&1 | FileCheck -check-prefix=UNSUPPORTED-TARGET %s
+
+/// Invalid option value
+// RUN: not %clang -target x86_64-linux-gnu -mtls-dialect=foo %s 2>&1 | FileCheck -check-prefix=INVALID-VALUE %s
+
+// TRAD: "-cc1" {{.*}}"-mtls-dialect=trad"
+// DESC: "-cc1" {{.*}}"-mtls-dialect=desc"
+// UNSUPPORTED-TARGET: error: unsupported option
+// INVALID-VALUE: error: invalid value 'foo' in '-mtls-dialect=foo'

@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 22, 2024

Note: codegen changes to support meaningful -mtls-dialect changes are introduced in #66915. This patch probably should land after those changes.

@@ -72,6 +72,11 @@ class CodeGenOptions : public CodeGenOptionsBase {
LocalExecTLSModel
};

enum TLSDialect {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer enum class (I think it has been more popular in other parts of Clang, perhaps CodeGen/Sema). TLSDialect::TraditionalDynamic and TLSDialect::TLSDesc

@@ -401,6 +401,8 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
Options.UniqueBasicBlockSectionNames =
CodeGenOpts.UniqueBasicBlockSectionNames;
Options.TLSSize = CodeGenOpts.TLSSize;
// TODO: Add correct codegen options in LLVM
// Options.TLSDesc = CodeGenOpts.getDefaultTLSDialect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be supported in this patch, otherwise there is no point in ignoring a rarely-used GCC option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@MaskRay
Copy link
Member

MaskRay commented Jan 24, 2024

Since TLSDialect only has 2 values and it's unlikely a future change will add new values, I think sticking with a boolean codegenopt suffices.
For x86, we can probably support "gnu" as "gnu2" (TLSDESC) is a desired feature. (I added lld support, but I did not know codegen well enough to add LLVM part...)

I think #79256 is a more complete patch, with these ideas taken into account..
It may be a good candidate for the release/18.x branch.

@ilovepi
Copy link
Contributor Author

ilovepi commented Jan 26, 2024

Abandoning in favor of #79256

@ilovepi ilovepi closed this Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants