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

Misc. rustc_codegen_ssa cleanups 🧹 #136439

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,22 +244,17 @@ pub fn each_linked_rlib(

fmts
} else {
for combination in info.dependency_formats.iter().combinations(2) {
let (ty1, list1) = &combination[0];
let (ty2, list2) = &combination[1];
if list1 != list2 {
return Err(errors::LinkRlibError::IncompatibleDependencyFormats {
ty1: format!("{ty1:?}"),
ty2: format!("{ty2:?}"),
list1: format!("{list1:?}"),
list2: format!("{list2:?}"),
});
}
}
if info.dependency_formats.is_empty() {
return Err(errors::LinkRlibError::MissingFormat);
let mut dep_formats = info.dependency_formats.iter();
let (ty1, list1) = dep_formats.next().ok_or(errors::LinkRlibError::MissingFormat)?;
if let Some((ty2, list2)) = dep_formats.find(|(_, list2)| list1 != *list2) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is equivalent, because if say the crate type 2 and 3 disagree it will no longer emit an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the == operator not transitive for this type?

Copy link
Member

Choose a reason for hiding this comment

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

It is. But say the dependency formats have 3 elements (where each element is a list), where the the second list and the last list are different. Previously, this was detected, because all combinations are checked, but now it is no longer detected because you only compare the first list against all other lists. The second and third lists are no longer compared.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do agree with @kadiwa4 on this. In your example if the second and last lists are different, then the first and last list will also be different, and it would still be caught.
But I might just be completely wrong on this, I dunno 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(assuming the second and first lists don't compare as equal)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's what I thought.
This is the (slightly shortened) definition of the transitivity property from the std docs:

a == b and b == c implies a == c

Now say a = dep_formats[1].1, b = dep_formats[0].1 and c = dep_formats[2].1. In the .find() method, the code checks that b == a and b == c. Assuming that find finishes without finding a mismatch, we now know that according to the transitivity property, a == c is true (i.e. dep_formats[1].1 == dep_formats[2].1).
(I am assuming that b == a is the same as a == b of course.)

return Err(errors::LinkRlibError::IncompatibleDependencyFormats {
ty1: format!("{ty1:?}"),
ty2: format!("{ty2:?}"),
list1: format!("{list1:?}"),
list2: format!("{list2:?}"),
});
}
info.dependency_formats.first().unwrap().1
list1
};

let used_dep_crates = info.used_crates.iter();
Expand Down Expand Up @@ -626,10 +621,9 @@ fn link_staticlib(

let mut all_rust_dylibs = vec![];
for &cnum in crates {
match fmts.get(cnum) {
Some(&Linkage::Dynamic) => {}
_ => continue,
}
let Some(Linkage::Dynamic) = fmts.get(cnum) else {
continue;
};
let crate_name = codegen_results.crate_info.crate_name[&cnum];
let used_crate_source = &codegen_results.crate_info.used_crate_source[&cnum];
if let Some((path, _)) = &used_crate_source.dylib {
Expand Down
15 changes: 8 additions & 7 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,10 +572,10 @@ fn produce_final_output_artifacts(
};

let copy_if_one_unit = |output_type: OutputType, keep_numbered: bool| {
if compiled_modules.modules.len() == 1 {
if let [module] = &compiled_modules.modules[..] {
// 1) Only one codegen unit. In this case it's no difficulty
// to copy `foo.0.x` to `foo.x`.
let module_name = Some(&compiled_modules.modules[0].name[..]);
let module_name = Some(&module.name[..]);
let path = crate_output.temp_path(output_type, module_name);
let output = crate_output.path(output_type);
if !output_type.is_text_output() && output.is_tty() {
Expand Down Expand Up @@ -707,8 +707,8 @@ fn produce_final_output_artifacts(
}

if sess.opts.json_artifact_notifications {
if compiled_modules.modules.len() == 1 {
compiled_modules.modules[0].for_each_output(|_path, ty| {
if let [module] = &compiled_modules.modules[..] {
module.for_each_output(|_path, ty| {
if sess.opts.output_types.contains_key(&ty) {
let descr = ty.shorthand();
// for single cgu file is renamed to drop cgu specific suffix
Expand Down Expand Up @@ -864,7 +864,7 @@ pub(crate) fn compute_per_cgu_lto_type(
// require LTO so the request for LTO is always unconditionally
// passed down to the backend, but we don't actually want to do
// anything about it yet until we've got a final product.
let is_rlib = sess_crate_types.len() == 1 && sess_crate_types[0] == CrateType::Rlib;
let is_rlib = matches!(sess_crate_types, [CrateType::Rlib]);

match sess_lto {
Lto::ThinLocal if !linker_does_lto && !is_allocator => ComputedLtoType::Thin,
Expand Down Expand Up @@ -1537,8 +1537,9 @@ fn start_executing_work<B: ExtraBackendMethods>(
// Spin up what work we can, only doing this while we've got available
// parallelism slots and work left to spawn.
if codegen_state != Aborted {
while !work_items.is_empty() && running_with_own_token < tokens.len() {
let (item, _) = work_items.pop().unwrap();
while running_with_own_token < tokens.len()
&& let Some((item, _)) = work_items.pop()
{
spawn_work(
&cgcx,
&mut llvm_start_time,
Expand Down
153 changes: 71 additions & 82 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::str::FromStr;

use rustc_abi::ExternAbi;
use rustc_ast::attr::list_contains_name;
use rustc_ast::expand::autodiff_attrs::{
AutoDiffAttrs, DiffActivity, DiffMode, valid_input_activity, valid_ret_activity,
};
Expand Down Expand Up @@ -385,24 +384,20 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
let segments =
set.path.segments.iter().map(|x| x.ident.name).collect::<Vec<_>>();
match segments.as_slice() {
[sym::arm, sym::a32] | [sym::arm, sym::t32] => {
if !tcx.sess.target.has_thumb_interworking {
struct_span_code_err!(
tcx.dcx(),
attr.span,
E0779,
"target does not support `#[instruction_set]`"
)
.emit();
None
} else if segments[1] == sym::a32 {
Some(InstructionSetAttr::ArmA32)
} else if segments[1] == sym::t32 {
Some(InstructionSetAttr::ArmT32)
} else {
unreachable!()
}
[sym::arm, sym::a32 | sym::t32]
if !tcx.sess.target.has_thumb_interworking =>
{
struct_span_code_err!(
tcx.dcx(),
attr.span,
E0779,
"target does not support `#[instruction_set]`"
)
.emit();
None
}
[sym::arm, sym::a32] => Some(InstructionSetAttr::ArmA32),
[sym::arm, sym::t32] => Some(InstructionSetAttr::ArmT32),
_ => {
struct_span_code_err!(
tcx.dcx(),
Expand Down Expand Up @@ -443,7 +438,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
&& let Some((sym::align, literal)) = item.singleton_lit_list()
{
rustc_attr_parsing::parse_alignment(&literal.kind)
.map_err(|msg| {
.inspect_err(|msg| {
struct_span_code_err!(
tcx.dcx(),
literal.span,
Expand Down Expand Up @@ -544,25 +539,27 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
}

if attr.is_word() {
InlineAttr::Hint
} else if let Some(ref items) = attr.meta_item_list() {
inline_span = Some(attr.span);
if items.len() != 1 {
struct_span_code_err!(tcx.dcx(), attr.span, E0534, "expected one argument").emit();
InlineAttr::None
} else if list_contains_name(items, sym::always) {
InlineAttr::Always
} else if list_contains_name(items, sym::never) {
InlineAttr::Never
} else {
struct_span_code_err!(tcx.dcx(), items[0].span(), E0535, "invalid argument")
.with_help("valid inline arguments are `always` and `never`")
.emit();
return InlineAttr::Hint;
}
let Some(ref items) = attr.meta_item_list() else {
return ia;
};

InlineAttr::None
}
inline_span = Some(attr.span);
let [item] = &items[..] else {
struct_span_code_err!(tcx.dcx(), attr.span, E0534, "expected one argument").emit();
return InlineAttr::None;
};
if item.has_name(sym::always) {
InlineAttr::Always
} else if item.has_name(sym::never) {
InlineAttr::Never
} else {
ia
struct_span_code_err!(tcx.dcx(), item.span(), E0535, "invalid argument")
.with_help("valid inline arguments are `always` and `never`")
.emit();

InlineAttr::None
}
});
codegen_fn_attrs.inline = attrs.iter().fold(codegen_fn_attrs.inline, |ia, attr| {
Expand Down Expand Up @@ -594,23 +591,25 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
let err = |sp, s| struct_span_code_err!(tcx.dcx(), sp, E0722, "{}", s).emit();
if attr.is_word() {
err(attr.span, "expected one argument");
ia
} else if let Some(ref items) = attr.meta_item_list() {
inline_span = Some(attr.span);
if items.len() != 1 {
err(attr.span, "expected one argument");
OptimizeAttr::Default
} else if list_contains_name(items, sym::size) {
OptimizeAttr::Size
} else if list_contains_name(items, sym::speed) {
OptimizeAttr::Speed
} else if list_contains_name(items, sym::none) {
OptimizeAttr::DoNotOptimize
} else {
err(items[0].span(), "invalid argument");
OptimizeAttr::Default
}
return ia;
}
let Some(ref items) = attr.meta_item_list() else {
return OptimizeAttr::Default;
};

inline_span = Some(attr.span);
let [item] = &items[..] else {
err(attr.span, "expected one argument");
return OptimizeAttr::Default;
};
if item.has_name(sym::size) {
OptimizeAttr::Size
} else if item.has_name(sym::speed) {
OptimizeAttr::Speed
} else if item.has_name(sym::none) {
OptimizeAttr::DoNotOptimize
} else {
err(item.span(), "invalid argument");
OptimizeAttr::Default
}
});
Expand Down Expand Up @@ -655,25 +654,20 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
// llvm/llvm-project#70563).
if !codegen_fn_attrs.target_features.is_empty()
&& matches!(codegen_fn_attrs.inline, InlineAttr::Always)
&& let Some(span) = inline_span
{
if let Some(span) = inline_span {
tcx.dcx().span_err(span, "cannot use `#[inline(always)]` with `#[target_feature]`");
}
tcx.dcx().span_err(span, "cannot use `#[inline(always)]` with `#[target_feature]`");
}

if !codegen_fn_attrs.no_sanitize.is_empty() && codegen_fn_attrs.inline.always() {
if let (Some(no_sanitize_span), Some(inline_span)) = (no_sanitize_span, inline_span) {
let hir_id = tcx.local_def_id_to_hir_id(did);
tcx.node_span_lint(
lint::builtin::INLINE_NO_SANITIZE,
hir_id,
no_sanitize_span,
|lint| {
lint.primary_message("`no_sanitize` will have no effect after inlining");
lint.span_note(inline_span, "inlining requested here");
},
)
}
if !codegen_fn_attrs.no_sanitize.is_empty()
&& codegen_fn_attrs.inline.always()
&& let (Some(no_sanitize_span), Some(inline_span)) = (no_sanitize_span, inline_span)
{
let hir_id = tcx.local_def_id_to_hir_id(did);
tcx.node_span_lint(lint::builtin::INLINE_NO_SANITIZE, hir_id, no_sanitize_span, |lint| {
lint.primary_message("`no_sanitize` will have no effect after inlining");
lint.span_note(inline_span, "inlining requested here");
})
}

// Weak lang items have the same semantics as "std internal" symbols in the
Expand Down Expand Up @@ -703,10 +697,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
// Any linkage to LLVM intrinsics for now forcibly marks them all as never
// unwinds since LLVM sometimes can't handle codegen which `invoke`s
// intrinsic functions.
if let Some(name) = &codegen_fn_attrs.link_name {
if name.as_str().starts_with("llvm.") {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NEVER_UNWIND;
}
if let Some(name) = &codegen_fn_attrs.link_name
&& name.as_str().starts_with("llvm.")
{
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NEVER_UNWIND;
}

if let Some(features) = check_tied_features(
Expand Down Expand Up @@ -767,18 +761,13 @@ fn should_inherit_track_caller(tcx: TyCtxt<'_>, def_id: DefId) -> bool {

fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &hir::Attribute) -> Option<u16> {
use rustc_ast::{LitIntType, LitKind, MetaItemLit};
let meta_item_list = attr.meta_item_list();
let meta_item_list = meta_item_list.as_deref();
let sole_meta_list = match meta_item_list {
Some([item]) => item.lit(),
Some(_) => {
tcx.dcx().emit_err(errors::InvalidLinkOrdinalNargs { span: attr.span });
return None;
}
_ => None,
let meta_item_list = attr.meta_item_list()?;
let [sole_meta_list] = &meta_item_list[..] else {
tcx.dcx().emit_err(errors::InvalidLinkOrdinalNargs { span: attr.span });
return None;
};
if let Some(MetaItemLit { kind: LitKind::Int(ordinal, LitIntType::Unsuffixed), .. }) =
sole_meta_list
sole_meta_list.lit()
{
// According to the table at
// https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#import-header, the
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,8 +990,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
});

// Split the rust-call tupled arguments off.
let (first_args, untuple) = if abi == ExternAbi::RustCall && !args.is_empty() {
let (tup, args) = args.split_last().unwrap();
let (first_args, untuple) = if abi == ExternAbi::RustCall
&& let Some((tup, args)) = args.split_last()
{
(args, Some(tup))
} else {
(args, None)
Expand Down
Loading