-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustc_target: add known safe s390x target features #127506
Changes from all commits
efcf35e
c940d52
366bc86
01e6e60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ use rustc_session::config::{PrintKind, PrintRequest}; | |
use rustc_session::Session; | ||
use rustc_span::symbol::Symbol; | ||
use rustc_target::spec::{MergeFunctions, PanicStrategy}; | ||
use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES; | ||
use rustc_target::target_features::{RUSTC_SPECIAL_FEATURES, RUSTC_SPECIFIC_FEATURES}; | ||
|
||
use std::ffi::{c_char, c_void, CStr, CString}; | ||
use std::fmt::Write; | ||
|
@@ -321,6 +321,10 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> { | |
} | ||
}) | ||
.filter(|feature| { | ||
// skip checking special features, as LLVM may not understands them | ||
if RUSTC_SPECIAL_FEATURES.contains(feature) { | ||
return true; | ||
} | ||
// check that all features in a given smallvec are enabled | ||
for llvm_feature in to_llvm_features(sess, feature) { | ||
let cstr = SmallCStr::new(llvm_feature); | ||
|
@@ -546,6 +550,7 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str | |
|
||
// -Ctarget-features | ||
let supported_features = sess.target.supported_target_features(); | ||
let (llvm_major, _, _) = get_version(); | ||
let mut featsmap = FxHashMap::default(); | ||
let feats = sess | ||
.opts | ||
|
@@ -604,6 +609,13 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec<Str | |
if RUSTC_SPECIFIC_FEATURES.contains(&feature) { | ||
return None; | ||
} | ||
|
||
// if the target-feature is "backchain" and LLVM version is greater than 18 | ||
// then we also need to add "+backchain" to the target-features attribute. | ||
// otherwise, we will only add the naked `backchain` attribute to the attribute-group. | ||
if feature == "backchain" && llvm_major < 18 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, this is due to older LLVM does not support adding this attribute to the target-features attribute. |
||
return None; | ||
} | ||
// ... otherwise though we run through `to_llvm_features` when | ||
// passing requests down to LLVM. This means that all in-language | ||
// features also work on the command line instead of having two | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes it so that
cfg!(target_feature = "backchain")
will always be true, even if it is not enabled. Is that the intended behavior? It seems very odd.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is to skip checking if LLVM supports this attribute. The logic for adding this attribute is somewhere else.
If you are unconvinced, check out this Godbolt playground: https://godbolt.org/z/77hrrsbo1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am convinced that the logic is wrong, see #129927.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I have no idea what I am supposed to look at in this Godbolt.^^ I know nothing about s390x or
backchain
, I am just noticing odd things in the rustc source code.