From feeb75e2639be481ef4428320b234d1e5b99e42d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 25 Apr 2020 21:45:21 +0300 Subject: [PATCH] rustc_target: Stop using "string typing" for TLS models Introduce `enum TlsModel` instead. --- src/librustc_codegen_llvm/back/write.rs | 7 --- src/librustc_codegen_llvm/context.rs | 23 +++----- src/librustc_codegen_llvm/lib.rs | 2 +- src/librustc_interface/tests.rs | 5 +- src/librustc_session/config.rs | 8 ++- src/librustc_session/options.rs | 15 +++++- src/librustc_session/session.rs | 6 ++- src/librustc_target/spec/cloudabi_base.rs | 4 +- src/librustc_target/spec/hermit_base.rs | 5 +- .../spec/hermit_kernel_base.rs | 5 +- src/librustc_target/spec/mod.rs | 54 +++++++++++++++++-- src/librustc_target/spec/wasm32_base.rs | 4 +- 12 files changed, 94 insertions(+), 44 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index bf79c5b593e..3ec7ef831b5 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -43,13 +43,6 @@ pub const CODE_GEN_MODEL_ARGS: &[(&str, llvm::CodeModel)] = &[ ("large", llvm::CodeModel::Large), ]; -pub const TLS_MODEL_ARGS: [(&str, llvm::ThreadLocalMode); 4] = [ - ("global-dynamic", llvm::ThreadLocalMode::GeneralDynamic), - ("local-dynamic", llvm::ThreadLocalMode::LocalDynamic), - ("initial-exec", llvm::ThreadLocalMode::InitialExec), - ("local-exec", llvm::ThreadLocalMode::LocalExec), -]; - pub fn llvm_err(handler: &rustc_errors::Handler, msg: &str) -> FatalError { match llvm::last_error() { Some(err) => handler.fatal(&format!("{}: {}", msg, err)), diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index df442609052..f868385ee86 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -21,7 +21,7 @@ use rustc_session::Session; use rustc_span::source_map::{Span, DUMMY_SP}; use rustc_span::symbol::Symbol; use rustc_target::abi::{HasDataLayout, LayoutOf, PointeeInfo, Size, TargetDataLayout, VariantIdx}; -use rustc_target::spec::{HasTargetSpec, RelocModel, Target}; +use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel}; use std::cell::{Cell, RefCell}; use std::ffi::CStr; @@ -87,19 +87,12 @@ pub struct CodegenCx<'ll, 'tcx> { local_gen_sym_counter: Cell, } -fn get_tls_model(sess: &Session) -> llvm::ThreadLocalMode { - let tls_model_arg = match sess.opts.debugging_opts.tls_model { - Some(ref s) => &s[..], - None => &sess.target.target.options.tls_model[..], - }; - - match crate::back::write::TLS_MODEL_ARGS.iter().find(|&&arg| arg.0 == tls_model_arg) { - Some(x) => x.1, - _ => { - sess.err(&format!("{:?} is not a valid TLS model", tls_model_arg)); - sess.abort_if_errors(); - bug!(); - } +fn to_llvm_tls_model(tls_model: TlsModel) -> llvm::ThreadLocalMode { + match tls_model { + TlsModel::GeneralDynamic => llvm::ThreadLocalMode::GeneralDynamic, + TlsModel::LocalDynamic => llvm::ThreadLocalMode::LocalDynamic, + TlsModel::InitialExec => llvm::ThreadLocalMode::InitialExec, + TlsModel::LocalExec => llvm::ThreadLocalMode::LocalExec, } } @@ -267,7 +260,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { let check_overflow = tcx.sess.overflow_checks(); - let tls_model = get_tls_model(&tcx.sess); + let tls_model = to_llvm_tls_model(tcx.sess.tls_model()); let (llcx, llmod) = (&*llvm_module.llcx, llvm_module.llmod()); diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index 330d6ea75d2..42302a56b41 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -216,7 +216,7 @@ impl CodegenBackend for LlvmCodegenBackend { } PrintRequest::TlsModels => { println!("Available TLS models:"); - for &(name, _) in back::write::TLS_MODEL_ARGS.iter() { + for name in &["global-dynamic", "local-dynamic", "initial-exec", "local-exec"] { println!(" {}", name); } println!(); diff --git a/src/librustc_interface/tests.rs b/src/librustc_interface/tests.rs index f0e7581b760..cee2e5b5bec 100644 --- a/src/librustc_interface/tests.rs +++ b/src/librustc_interface/tests.rs @@ -14,7 +14,8 @@ use rustc_session::{build_session, Session}; use rustc_span::edition::{Edition, DEFAULT_EDITION}; use rustc_span::symbol::sym; use rustc_span::SourceFileHashAlgorithm; -use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelocModel, RelroLevel}; +use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy}; +use rustc_target::spec::{RelocModel, RelroLevel, TlsModel}; use std::collections::{BTreeMap, BTreeSet}; use std::iter::FromIterator; use std::path::PathBuf; @@ -567,7 +568,7 @@ fn test_debugging_options_tracking_hash() { tracked!(symbol_mangling_version, SymbolManglingVersion::V0); tracked!(teach, true); tracked!(thinlto, Some(true)); - tracked!(tls_model, Some(String::from("tls model"))); + tracked!(tls_model, Some(TlsModel::GeneralDynamic)); tracked!(treat_err_as_bug, Some(1)); tracked!(unleash_the_miri_inside_of_you, true); tracked!(verify_llvm_ir, true); diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index 1ab02f84c11..0dfc391d9cd 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -1315,10 +1315,6 @@ fn collect_print_requests( prints.push(PrintRequest::CodeModels); cg.code_model = None; } - if dopts.tls_model.as_ref().map_or(false, |s| s == "help") { - prints.push(PrintRequest::TlsModels); - dopts.tls_model = None; - } prints.extend(matches.opt_strs("print").into_iter().map(|s| match &*s { "crate-name" => PrintRequest::CrateName, @@ -2001,7 +1997,8 @@ crate mod dep_tracking { use crate::utils::NativeLibraryKind; use rustc_feature::UnstableFeatures; use rustc_span::edition::Edition; - use rustc_target::spec::{MergeFunctions, PanicStrategy, RelocModel, RelroLevel, TargetTriple}; + use rustc_target::spec::{MergeFunctions, PanicStrategy, RelocModel}; + use rustc_target::spec::{RelroLevel, TargetTriple, TlsModel}; use std::collections::hash_map::DefaultHasher; use std::collections::BTreeMap; use std::hash::Hash; @@ -2050,6 +2047,7 @@ crate mod dep_tracking { impl_dep_tracking_hash_via_hash!(Option>); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); + impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 2279c16748c..5b983d1105d 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -6,7 +6,8 @@ use crate::search_paths::SearchPath; use crate::utils::NativeLibraryKind; use rustc_target::spec::TargetTriple; -use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelocModel, RelroLevel}; +use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy}; +use rustc_target::spec::{RelocModel, RelroLevel, TlsModel}; use rustc_feature::UnstableFeatures; use rustc_span::edition::Edition; @@ -267,6 +268,8 @@ macro_rules! options { pub const parse_src_file_hash: &str = "either `md5` or `sha1`"; pub const parse_relocation_model: &str = "one of supported relocation models (`rustc --print relocation-models`)"; + pub const parse_tls_model: &str = + "one of supported TLS models (`rustc --print tls-models`)"; } #[allow(dead_code)] @@ -606,6 +609,14 @@ macro_rules! options { true } + fn parse_tls_model(slot: &mut Option, v: Option<&str>) -> bool { + match v.and_then(|s| TlsModel::from_str(s).ok()) { + Some(tls_model) => *slot = Some(tls_model), + _ => return false, + } + true + } + fn parse_symbol_mangling_version( slot: &mut SymbolManglingVersion, v: Option<&str>, @@ -977,7 +988,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "measure time of each LLVM pass (default: no)"), time_passes: bool = (false, parse_bool, [UNTRACKED], "measure time of each rustc pass (default: no)"), - tls_model: Option = (None, parse_opt_string, [TRACKED], + tls_model: Option = (None, parse_tls_model, [TRACKED], "choose the TLS model to use (`rustc --print tls-models` for details)"), trace_macros: bool = (false, parse_bool, [UNTRACKED], "for every macro invocation, print its name and arguments (default: no)"), diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 226d9392095..42f9a8d6b05 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -22,7 +22,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticId, ErrorReported use rustc_span::edition::Edition; use rustc_span::source_map::{self, FileLoader, MultiSpan, RealFileLoader, SourceMap, Span}; use rustc_span::SourceFileHashAlgorithm; -use rustc_target::spec::{PanicStrategy, RelocModel, RelroLevel, Target, TargetTriple}; +use rustc_target::spec::{PanicStrategy, RelocModel, RelroLevel, Target, TargetTriple, TlsModel}; use std::cell::{self, RefCell}; use std::env; @@ -588,6 +588,10 @@ impl Session { self.opts.cg.relocation_model.unwrap_or(self.target.target.options.relocation_model) } + pub fn tls_model(&self) -> TlsModel { + self.opts.debugging_opts.tls_model.unwrap_or(self.target.target.options.tls_model) + } + pub fn must_not_eliminate_frame_pointers(&self) -> bool { // "mcount" function relies on stack pointer. // See . diff --git a/src/librustc_target/spec/cloudabi_base.rs b/src/librustc_target/spec/cloudabi_base.rs index 53af9dcc186..3659c9ecdfc 100644 --- a/src/librustc_target/spec/cloudabi_base.rs +++ b/src/librustc_target/spec/cloudabi_base.rs @@ -1,4 +1,4 @@ -use crate::spec::{LinkArgs, LinkerFlavor, RelroLevel, TargetOptions}; +use crate::spec::{LinkArgs, LinkerFlavor, RelroLevel, TargetOptions, TlsModel}; pub fn opts() -> TargetOptions { let mut args = LinkArgs::new(); @@ -29,7 +29,7 @@ pub fn opts() -> TargetOptions { // (Global Offset Table) to obtain the effective address of a // thread-local variable. Using a GOT is useful only when doing // dynamic linking. - tls_model: "local-exec".to_string(), + tls_model: TlsModel::LocalExec, relro_level: RelroLevel::Full, ..Default::default() } diff --git a/src/librustc_target/spec/hermit_base.rs b/src/librustc_target/spec/hermit_base.rs index cb12055290e..18fb2aa3d56 100644 --- a/src/librustc_target/spec/hermit_base.rs +++ b/src/librustc_target/spec/hermit_base.rs @@ -1,4 +1,5 @@ -use crate::spec::{LinkArgs, LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, TargetOptions}; +use crate::spec::{LinkArgs, LinkerFlavor, LldFlavor, PanicStrategy}; +use crate::spec::{RelocModel, TargetOptions, TlsModel}; pub fn opts() -> TargetOptions { let mut pre_link_args = LinkArgs::new(); @@ -17,7 +18,7 @@ pub fn opts() -> TargetOptions { position_independent_executables: true, relocation_model: RelocModel::Static, target_family: None, - tls_model: "initial-exec".to_string(), + tls_model: TlsModel::InitialExec, ..Default::default() } } diff --git a/src/librustc_target/spec/hermit_kernel_base.rs b/src/librustc_target/spec/hermit_kernel_base.rs index 11599fda409..7f2dada714d 100644 --- a/src/librustc_target/spec/hermit_kernel_base.rs +++ b/src/librustc_target/spec/hermit_kernel_base.rs @@ -1,4 +1,5 @@ -use crate::spec::{LinkArgs, LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, TargetOptions}; +use crate::spec::{LinkArgs, LinkerFlavor, LldFlavor, PanicStrategy}; +use crate::spec::{RelocModel, TargetOptions, TlsModel}; pub fn opts() -> TargetOptions { let mut pre_link_args = LinkArgs::new(); @@ -18,7 +19,7 @@ pub fn opts() -> TargetOptions { position_independent_executables: true, relocation_model: RelocModel::Static, target_family: None, - tls_model: "initial-exec".to_string(), + tls_model: TlsModel::InitialExec, ..Default::default() } } diff --git a/src/librustc_target/spec/mod.rs b/src/librustc_target/spec/mod.rs index 77fc78e8148..e853c07632f 100644 --- a/src/librustc_target/spec/mod.rs +++ b/src/librustc_target/spec/mod.rs @@ -305,6 +305,42 @@ impl ToJson for RelocModel { } } +#[derive(Clone, Copy, PartialEq, Hash, Debug)] +pub enum TlsModel { + GeneralDynamic, + LocalDynamic, + InitialExec, + LocalExec, +} + +impl FromStr for TlsModel { + type Err = (); + + fn from_str(s: &str) -> Result { + Ok(match s { + // Note the difference "general" vs "global" difference. The model name is "general", + // but the user-facing option name is "global" for consistency with other compilers. + "global-dynamic" => TlsModel::GeneralDynamic, + "local-dynamic" => TlsModel::LocalDynamic, + "initial-exec" => TlsModel::InitialExec, + "local-exec" => TlsModel::LocalExec, + _ => return Err(()), + }) + } +} + +impl ToJson for TlsModel { + fn to_json(&self) -> Json { + match *self { + TlsModel::GeneralDynamic => "global-dynamic", + TlsModel::LocalDynamic => "local-dynamic", + TlsModel::InitialExec => "initial-exec", + TlsModel::LocalExec => "local-exec", + } + .to_json() + } +} + pub enum LoadTargetError { BuiltinTargetNotFound(String), Other(String), @@ -660,7 +696,7 @@ pub struct TargetOptions { pub code_model: Option, /// TLS model to use. Options are "global-dynamic" (default), "local-dynamic", "initial-exec" /// and "local-exec". This is similar to the -ftls-model option in GCC/Clang. - pub tls_model: String, + pub tls_model: TlsModel, /// Do not emit code that uses the "red zone", if the ABI has one. Defaults to false. pub disable_redzone: bool, /// Eliminate frame pointers from stack frames if possible. Defaults to true. @@ -863,7 +899,7 @@ impl Default for TargetOptions { executables: false, relocation_model: RelocModel::Pic, code_model: None, - tls_model: "global-dynamic".to_string(), + tls_model: TlsModel::GeneralDynamic, disable_redzone: false, eliminate_frame_pointer: true, function_sections: true, @@ -1060,6 +1096,18 @@ impl Target { Some(Ok(())) })).unwrap_or(Ok(())) } ); + ($key_name:ident, TlsModel) => ( { + let name = (stringify!($key_name)).replace("_", "-"); + obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| { + match s.parse::() { + Ok(tls_model) => base.options.$key_name = tls_model, + _ => return Some(Err(format!("'{}' is not a valid TLS model. \ + Run `rustc --print tls-models` to \ + see the list of supported values.", s))), + } + Some(Ok(())) + })).unwrap_or(Ok(())) + } ); ($key_name:ident, PanicStrategy) => ( { let name = (stringify!($key_name)).replace("_", "-"); obj.find(&name[..]).and_then(|o| o.as_string().and_then(|s| { @@ -1200,7 +1248,7 @@ impl Target { key!(executables, bool); key!(relocation_model, RelocModel)?; key!(code_model, optional); - key!(tls_model); + key!(tls_model, TlsModel)?; key!(disable_redzone, bool); key!(eliminate_frame_pointer, bool); key!(function_sections, bool); diff --git a/src/librustc_target/spec/wasm32_base.rs b/src/librustc_target/spec/wasm32_base.rs index 08bade2abf4..bb19b9d00e8 100644 --- a/src/librustc_target/spec/wasm32_base.rs +++ b/src/librustc_target/spec/wasm32_base.rs @@ -1,4 +1,4 @@ -use super::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, TargetOptions}; +use super::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, TargetOptions, TlsModel}; use std::collections::BTreeMap; pub fn options() -> TargetOptions { @@ -138,7 +138,7 @@ pub fn options() -> TargetOptions { // `has_elf_tls`) and we need to get it to work by specifying // `local-exec` as that's all that's implemented in LLVM today for wasm. has_elf_tls: true, - tls_model: "local-exec".to_string(), + tls_model: TlsModel::LocalExec, // gdb scripts don't work on wasm blobs emit_debug_gdb_scripts: false,