]> git.lizzy.rs Git - rust.git/commitdiff
rustc: Tweak `#[target_feature]` syntax
authorAlex Crichton <alex@alexcrichton.com>
Fri, 5 Jan 2018 21:26:26 +0000 (13:26 -0800)
committerAlex Crichton <alex@alexcrichton.com>
Sat, 13 Jan 2018 18:07:18 +0000 (10:07 -0800)
This is an implementation of the `#[target_feature]` syntax-related changes of
[RFC 2045][rfc]. Notably two changes have been implemented:

* The new syntax is `#[target_feature(enable = "..")]` instead of
  `#[target_feature = "+.."]`. The `enable` key is necessary instead of the `+`
  to indicate that a feature is being enabled, and a sub-list is used for
  possible expansion in the future. Additionally within this syntax the feature
  names being enabled are now whitelisted against a known set of target feature
  names that we know about.

* The `#[target_feature]` attribute can only be applied to unsafe functions. It
  was decided in the RFC that invoking an instruction possibly not defined for
  the current processor is undefined behavior, so to enable this feature for now
  it requires an `unsafe` intervention.

[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2045-target-feature.md

src/librustc/dep_graph/dep_node.rs
src/librustc/ty/maps/config.rs
src/librustc/ty/maps/mod.rs
src/librustc/ty/maps/plumbing.rs
src/librustc_trans/attributes.rs
src/librustc_trans/callee.rs
src/librustc_trans/lib.rs
src/librustc_trans/llvm_util.rs
src/librustc_trans/trans_item.rs
src/test/ui/target-feature-wrong.rs [new file with mode: 0644]
src/test/ui/target-feature-wrong.stderr [new file with mode: 0644]

index ec52c6cf57bf76ca4ff2230de54da685f8ca5342..eda7ddb161f1273b09273c6174d1b4be277d2c41 100644 (file)
@@ -635,6 +635,10 @@ pub fn fingerprint_needed_for_crate_hash(self) -> bool {
     [] Null,
 
     [] SubstituteNormalizeAndTestPredicates { key: (DefId, &'tcx Substs<'tcx>) },
+
+    [input] TargetFeaturesWhitelist,
+    [] TargetFeaturesEnabled(DefId),
+
 );
 
 trait DepNodeParams<'a, 'gcx: 'tcx + 'a, 'tcx: 'a> : fmt::Debug {
index 881c59e05aac6268f014ee764e04c53b084e991c..8dedcb24c2fb61a8b50dca7a9d2232ab7941ae98 100644 (file)
@@ -631,6 +631,12 @@ fn describe(tcx: TyCtxt, key: (DefId, &'tcx Substs<'tcx>)) -> String {
     }
 }
 
+impl<'tcx> QueryDescription<'tcx> for queries::target_features_whitelist<'tcx> {
+    fn describe(_tcx: TyCtxt, _: CrateNum) -> String {
+        format!("looking up the whitelist of target features")
+    }
+}
+
 macro_rules! impl_disk_cacheable_query(
     ($query_name:ident, |$key:tt| $cond:expr) => {
         impl<'tcx> QueryDescription<'tcx> for queries::$query_name<'tcx> {
index bd8c22aab931624714806dca0fde8bf994154995..51cae9e13f8838a1061fa535df83f2512f8ccaa2 100644 (file)
 
     [] fn substitute_normalize_and_test_predicates:
         substitute_normalize_and_test_predicates_node((DefId, &'tcx Substs<'tcx>)) -> bool,
+
+    [] fn target_features_whitelist:
+        target_features_whitelist_node(CrateNum) -> Rc<FxHashSet<String>>,
+    [] fn target_features_enabled: TargetFeaturesEnabled(DefId) -> Rc<Vec<String>>,
+
 }
 
 //////////////////////////////////////////////////////////////////////
@@ -508,3 +513,7 @@ fn substitute_normalize_and_test_predicates_node<'tcx>(key: (DefId, &'tcx Substs
                                             -> DepConstructor<'tcx> {
     DepConstructor::SubstituteNormalizeAndTestPredicates { key }
 }
+
+fn target_features_whitelist_node<'tcx>(_: CrateNum) -> DepConstructor<'tcx> {
+    DepConstructor::TargetFeaturesWhitelist
+}
index dd8b8a2e5da8268f5e74c73a13a757e853ce41dd..4f6f1ab0ee2dd1dda937cbac813771f3311a877e 100644 (file)
@@ -918,6 +918,9 @@ macro_rules! force {
         }
         DepKind::IsTranslatedFunction => { force!(is_translated_function, def_id!()); }
         DepKind::OutputFilenames => { force!(output_filenames, LOCAL_CRATE); }
+
+        DepKind::TargetFeaturesWhitelist => { force!(target_features_whitelist, LOCAL_CRATE); }
+        DepKind::TargetFeaturesEnabled => { force!(target_features_enabled, def_id!()); }
     }
 
     true
index 745aa0da82900d1681a7432a9afd6745c4ce5c0b..f3105e03523a3ba657f451d4a0885a9dcc6d7bef 100644 (file)
 //! Set and unset common attributes on LLVM values.
 
 use std::ffi::{CStr, CString};
+use std::rc::Rc;
 
+use rustc::hir::Unsafety;
+use rustc::hir::def_id::{DefId, LOCAL_CRATE};
 use rustc::session::config::Sanitizer;
+use rustc::ty::TyCtxt;
+use rustc::ty::maps::Providers;
+use rustc_data_structures::fx::FxHashSet;
 
 use llvm::{self, Attribute, ValueRef};
 use llvm::AttributePlace::Function;
+use llvm_util;
 pub use syntax::attr::{self, InlineAttr};
 use syntax::ast;
 use context::CrateContext;
@@ -94,23 +101,16 @@ pub fn set_probestack(ccx: &CrateContext, llfn: ValueRef) {
 
 /// Composite function which sets LLVM attributes for function depending on its AST (#[attribute])
 /// attributes.
-pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRef) {
+pub fn from_fn_attrs(ccx: &CrateContext, llfn: ValueRef, id: DefId) {
     use syntax::attr::*;
-    inline(llfn, find_inline_attr(Some(ccx.sess().diagnostic()), attrs));
+    let attrs = ccx.tcx().get_attrs(id);
+    inline(llfn, find_inline_attr(Some(ccx.sess().diagnostic()), &attrs));
 
     set_frame_pointer_elimination(ccx, llfn);
     set_probestack(ccx, llfn);
-    let mut target_features = vec![];
-    for attr in attrs {
-        if attr.check_name("target_feature") {
-            if let Some(val) = attr.value_str() {
-                for feat in val.as_str().split(",").map(|f| f.trim()) {
-                    if !feat.is_empty() && !feat.contains('\0') {
-                        target_features.push(feat.to_string());
-                    }
-                }
-            }
-        } else if attr.check_name("cold") {
+
+    for attr in attrs.iter() {
+        if attr.check_name("cold") {
             Attribute::Cold.apply_llfn(Function, llfn);
         } else if attr.check_name("naked") {
             naked(llfn, true);
@@ -123,6 +123,8 @@ pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRe
             unwind(llfn, false);
         }
     }
+
+    let target_features = ccx.tcx().target_features_enabled(id);
     if !target_features.is_empty() {
         let val = CString::new(target_features.join(",")).unwrap();
         llvm::AddFunctionAttrStringValue(
@@ -134,3 +136,97 @@ pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRe
 fn cstr(s: &'static str) -> &CStr {
     CStr::from_bytes_with_nul(s.as_bytes()).expect("null-terminated string")
 }
+
+pub fn provide(providers: &mut Providers) {
+    providers.target_features_whitelist = |tcx, cnum| {
+        assert_eq!(cnum, LOCAL_CRATE);
+        Rc::new(llvm_util::target_feature_whitelist(tcx.sess)
+            .iter()
+            .map(|c| c.to_str().unwrap().to_string())
+            .collect())
+    };
+
+    providers.target_features_enabled = |tcx, id| {
+        let whitelist = tcx.target_features_whitelist(LOCAL_CRATE);
+        let mut target_features = Vec::new();
+        for attr in tcx.get_attrs(id).iter() {
+            if !attr.check_name("target_feature") {
+                continue
+            }
+            if let Some(val) = attr.value_str() {
+                for feat in val.as_str().split(",").map(|f| f.trim()) {
+                    if !feat.is_empty() && !feat.contains('\0') {
+                        target_features.push(feat.to_string());
+                    }
+                }
+                let msg = "#[target_feature = \"..\"] is deprecated and will \
+                           eventually be removed, use \
+                           #[target_feature(enable = \"..\")] instead";
+                tcx.sess.span_warn(attr.span, &msg);
+                continue
+            }
+
+            if tcx.fn_sig(id).unsafety() == Unsafety::Normal {
+                let msg = "#[target_feature(..)] can only be applied to \
+                           `unsafe` function";
+                tcx.sess.span_err(attr.span, msg);
+            }
+            from_target_feature(tcx, attr, &whitelist, &mut target_features);
+        }
+        Rc::new(target_features)
+    };
+}
+
+fn from_target_feature(
+    tcx: TyCtxt,
+    attr: &ast::Attribute,
+    whitelist: &FxHashSet<String>,
+    target_features: &mut Vec<String>,
+) {
+    let list = match attr.meta_item_list() {
+        Some(list) => list,
+        None => {
+            let msg = "#[target_feature] attribute must be of the form \
+                       #[target_feature(..)]";
+            tcx.sess.span_err(attr.span, &msg);
+            return
+        }
+    };
+
+    for item in list {
+        if !item.check_name("enable") {
+            let msg = "#[target_feature(..)] only accepts sub-keys of `enable` \
+                       currently";
+            tcx.sess.span_err(item.span, &msg);
+            continue
+        }
+        let value = match item.value_str() {
+            Some(list) => list,
+            None => {
+                let msg = "#[target_feature] attribute must be of the form \
+                           #[target_feature(enable = \"..\")]";
+                tcx.sess.span_err(item.span, &msg);
+                continue
+            }
+        };
+        let value = value.as_str();
+        for feature in value.split(',') {
+            if whitelist.contains(feature) {
+                target_features.push(format!("+{}", feature));
+                continue
+            }
+
+            let msg = format!("the feature named `{}` is not valid for \
+                               this target", feature);
+            let mut err = tcx.sess.struct_span_err(item.span, &msg);
+
+            if feature.starts_with("+") {
+                let valid = whitelist.contains(&feature[1..]);
+                if valid {
+                    err.help("consider removing the leading `+` in the feature name");
+                }
+            }
+            err.emit();
+        }
+    }
+}
index 0a0f2615a1bd162242051521a0a71ae9adface6a..ccbc6620ffe09b0755a488e3e254a1af06da0683 100644 (file)
@@ -99,8 +99,7 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
         if instance.def.is_inline(tcx) {
             attributes::inline(llfn, attributes::InlineAttr::Hint);
         }
-        let attrs = instance.def.attrs(ccx.tcx());
-        attributes::from_fn_attrs(ccx, &attrs, llfn);
+        attributes::from_fn_attrs(ccx, llfn, instance.def.def_id());
 
         let instance_def_id = instance.def_id();
 
index ee08a7f1ec471c53bceaef2b6033e3e2cf8abdb5..974c268749b859a5620059aea0f6e6846963aaa7 100644 (file)
@@ -167,6 +167,7 @@ fn provide(providers: &mut ty::maps::Providers) {
         back::symbol_names::provide(providers);
         back::symbol_export::provide(providers);
         base::provide(providers);
+        attributes::provide(providers);
     }
 
     fn provide_extern(providers: &mut ty::maps::Providers) {
index b3d0b574d1d432bd08f9e7f7101564b9baf78bd1..8112a9eeab1ef4d8d622bd5dcfc5a561eccb153e 100644 (file)
@@ -13,8 +13,8 @@
 use llvm;
 use rustc::session::Session;
 use rustc::session::config::PrintRequest;
-use libc::{c_int, c_char};
-use std::ffi::CString;
+use libc::c_int;
+use std::ffi::{CStr, CString};
 
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Once;
@@ -97,8 +97,18 @@ unsafe fn configure_llvm(sess: &Session) {
 const MIPS_WHITELIST: &'static [&'static str] = &["msa\0"];
 
 pub fn target_features(sess: &Session) -> Vec<Symbol> {
+    let whitelist = target_feature_whitelist(sess);
     let target_machine = create_target_machine(sess);
+    let mut features = Vec::new();
+    for feat in whitelist {
+        if unsafe { llvm::LLVMRustHasFeature(target_machine, feat.as_ptr()) } {
+            features.push(Symbol::intern(feat.to_str().unwrap()));
+        }
+    }
+    features
+}
 
+pub fn target_feature_whitelist(sess: &Session) -> Vec<&CStr> {
     let whitelist = match &*sess.target.target.arch {
         "arm" => ARM_WHITELIST,
         "aarch64" => AARCH64_WHITELIST,
@@ -108,15 +118,9 @@ pub fn target_features(sess: &Session) -> Vec<Symbol> {
         "powerpc" | "powerpc64" => POWERPC_WHITELIST,
         _ => &[],
     };
-
-    let mut features = Vec::new();
-    for feat in whitelist {
-        assert_eq!(feat.chars().last(), Some('\0'));
-        if unsafe { llvm::LLVMRustHasFeature(target_machine, feat.as_ptr() as *const c_char) } {
-            features.push(Symbol::intern(&feat[..feat.len() - 1]));
-        }
-    }
-    features
+    whitelist.iter().map(|m| {
+        CStr::from_bytes_with_nul(m.as_bytes()).unwrap()
+    }).collect()
 }
 
 pub fn print_version() {
index 31d8e092c4ae49a05486d78c11ad89d3a054cbce..fa6a42e062d9fb5645f755a78b4f4ccc3222e995 100644 (file)
@@ -199,7 +199,7 @@ fn predefine_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
     if instance.def.is_inline(ccx.tcx()) {
         attributes::inline(lldecl, attributes::InlineAttr::Hint);
     }
-    attributes::from_fn_attrs(ccx, &attrs, lldecl);
+    attributes::from_fn_attrs(ccx, lldecl, instance.def.def_id());
 
     ccx.instances().borrow_mut().insert(instance, lldecl);
 }
diff --git a/src/test/ui/target-feature-wrong.rs b/src/test/ui/target-feature-wrong.rs
new file mode 100644 (file)
index 0000000..df24035
--- /dev/null
@@ -0,0 +1,35 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// ignore-arm
+// ignore-aarch64
+
+#![feature(target_feature)]
+
+#[target_feature = "+sse2"]
+//~^ WARN: deprecated
+#[target_feature(enable = "foo")]
+//~^ ERROR: not valid for this target
+#[target_feature(bar)]
+//~^ ERROR: only accepts sub-keys
+#[target_feature(disable = "baz")]
+//~^ ERROR: only accepts sub-keys
+unsafe fn foo() {}
+
+#[target_feature(enable = "sse2")]
+//~^ ERROR: can only be applied to `unsafe` function
+fn bar() {}
+
+fn main() {
+    unsafe {
+        foo();
+        bar();
+    }
+}
diff --git a/src/test/ui/target-feature-wrong.stderr b/src/test/ui/target-feature-wrong.stderr
new file mode 100644 (file)
index 0000000..0cbfeb3
--- /dev/null
@@ -0,0 +1,32 @@
+warning: #[target_feature = ".."] is deprecated and will eventually be removed, use #[target_feature(enable = "..")] instead
+  --> $DIR/target-feature-wrong.rs:16:1
+   |
+16 | #[target_feature = "+sse2"]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: the feature named `foo` is not valid for this target
+  --> $DIR/target-feature-wrong.rs:18:18
+   |
+18 | #[target_feature(enable = "foo")]
+   |                  ^^^^^^^^^^^^^^
+
+error: #[target_feature(..)] only accepts sub-keys of `enable` currently
+  --> $DIR/target-feature-wrong.rs:20:18
+   |
+20 | #[target_feature(bar)]
+   |                  ^^^
+
+error: #[target_feature(..)] only accepts sub-keys of `enable` currently
+  --> $DIR/target-feature-wrong.rs:22:18
+   |
+22 | #[target_feature(disable = "baz")]
+   |                  ^^^^^^^^^^^^^^^
+
+error: #[target_feature(..)] can only be applied to `unsafe` function
+  --> $DIR/target-feature-wrong.rs:26:1
+   |
+26 | #[target_feature(enable = "sse2")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+