diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 720970bbaf93f..9fbfa0a9f7654 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -24,12 +24,13 @@ use rustc_macros::{ use rustc_middle::metadata::ModChild; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs; use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile; +use rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs; use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo}; use rustc_middle::middle::lib_features::FeatureStability; use rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault; use rustc_middle::mir; use rustc_middle::ty::fast_reject::SimplifiedType; -use rustc_middle::ty::{self, DeducedParamAttrs, Ty, TyCtxt, UnusedGenericParams}; +use rustc_middle::ty::{self, Ty, TyCtxt, UnusedGenericParams}; use rustc_middle::util::Providers; use rustc_serialize::opaque::FileEncoder; use rustc_session::config::{SymbolManglingVersion, TargetModifier}; diff --git a/compiler/rustc_metadata/src/rmeta/parameterized.rs b/compiler/rustc_metadata/src/rmeta/parameterized.rs index 4b2dc2c814e57..733e33f310a67 100644 --- a/compiler/rustc_metadata/src/rmeta/parameterized.rs +++ b/compiler/rustc_metadata/src/rmeta/parameterized.rs @@ -97,6 +97,7 @@ trivially_parameterized_over_tcx! { rustc_middle::metadata::ModChild, rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs, rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile, + rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs, rustc_middle::middle::exported_symbols::SymbolExportInfo, rustc_middle::middle::lib_features::FeatureStability, rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault, @@ -105,7 +106,6 @@ trivially_parameterized_over_tcx! { rustc_middle::ty::AssocContainer, rustc_middle::ty::AsyncDestructor, rustc_middle::ty::Asyncness, - rustc_middle::ty::DeducedParamAttrs, rustc_middle::ty::Destructor, rustc_middle::ty::Generics, rustc_middle::ty::ImplTraitInTraitData, diff --git a/compiler/rustc_middle/src/middle/deduced_param_attrs.rs b/compiler/rustc_middle/src/middle/deduced_param_attrs.rs new file mode 100644 index 0000000000000..f9177a067b413 --- /dev/null +++ b/compiler/rustc_middle/src/middle/deduced_param_attrs.rs @@ -0,0 +1,67 @@ +use rustc_macros::{Decodable, Encodable, HashStable}; + +use crate::ty::{Ty, TyCtxt, TypingEnv}; + +/// Flags that dictate how a parameter is mutated. If the flags are empty, the param is +/// read-only. If non-empty, it is read-only with conditions. +#[derive(Clone, Copy, PartialEq, Debug, Decodable, Encodable, HashStable)] +pub struct DeducedReadOnlyParam(u8); + +bitflags::bitflags! { + impl DeducedReadOnlyParam: u8 { + /// This parameter is dropped. It is read-only if `!needs_drop`. + const IF_NO_DROP = 1 << 0; + /// This parameter is borrowed. It is read-only if `Freeze`. + const IF_FREEZE = 1 << 1; + /// This parameter is mutated. It is never read-only. + const MUTATED = 1 << 2; + } +} + +/// Parameter attributes that can only be determined by examining the body of a function instead +/// of just its signature. +/// +/// These can be useful for optimization purposes when a function is directly called. We compute +/// them and store them into the crate metadata so that downstream crates can make use of them. +/// +/// Right now, we only have `read_only`, but `no_capture` and `no_alias` might be useful in the +/// future. +#[derive(Clone, Copy, PartialEq, Debug, Decodable, Encodable, HashStable)] +pub struct DeducedParamAttrs { + /// The parameter is marked immutable in the function. + pub read_only: DeducedReadOnlyParam, +} + +// By default, consider the parameters to be mutated. +impl Default for DeducedParamAttrs { + #[inline] + fn default() -> DeducedParamAttrs { + DeducedParamAttrs { read_only: DeducedReadOnlyParam::MUTATED } + } +} + +impl DeducedParamAttrs { + #[inline] + pub fn is_default(self) -> bool { + self.read_only.contains(DeducedReadOnlyParam::MUTATED) + } + + pub fn read_only<'tcx>( + &self, + tcx: TyCtxt<'tcx>, + typing_env: TypingEnv<'tcx>, + ty: Ty<'tcx>, + ) -> bool { + let read_only = self.read_only; + if read_only.contains(DeducedReadOnlyParam::MUTATED) { + return false; + } + if read_only.contains(DeducedReadOnlyParam::IF_NO_DROP) && ty.needs_drop(tcx, typing_env) { + return false; + } + if read_only.contains(DeducedReadOnlyParam::IF_FREEZE) && !ty.is_freeze(tcx, typing_env) { + return false; + } + true + } +} diff --git a/compiler/rustc_middle/src/middle/mod.rs b/compiler/rustc_middle/src/middle/mod.rs index 6ca1e6207042a..9091d492c26b2 100644 --- a/compiler/rustc_middle/src/middle/mod.rs +++ b/compiler/rustc_middle/src/middle/mod.rs @@ -1,5 +1,6 @@ pub mod codegen_fn_attrs; pub mod debugger_visualizer; +pub mod deduced_param_attrs; pub mod dependency_format; pub mod exported_symbols; pub mod lang_items; diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index cad10fcfb0101..5b75609394ea8 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -313,6 +313,7 @@ trivial! { rustc_hir::Stability, rustc_hir::Upvar, rustc_index::bit_set::FiniteBitSet, + rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs, rustc_middle::middle::dependency_format::Linkage, rustc_middle::middle::exported_symbols::SymbolExportInfo, rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault, @@ -336,7 +337,6 @@ trivial! { rustc_middle::ty::AsyncDestructor, rustc_middle::ty::BoundVariableKind, rustc_middle::ty::AnonConstKind, - rustc_middle::ty::DeducedParamAttrs, rustc_middle::ty::Destructor, rustc_middle::ty::fast_reject::SimplifiedType, rustc_middle::ty::ImplPolarity, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 448c26ef31813..40d2d53e61cc3 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -107,6 +107,7 @@ use crate::lint::LintExpectation; use crate::metadata::ModChild; use crate::middle::codegen_fn_attrs::CodegenFnAttrs; use crate::middle::debugger_visualizer::DebuggerVisualizerFile; +use crate::middle::deduced_param_attrs::DeducedParamAttrs; use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo}; use crate::middle::lib_features::LibFeatures; use crate::middle::privacy::EffectiveVisibilities; @@ -2657,7 +2658,7 @@ rustc_queries! { return_result_from_ensure_ok } - query deduced_param_attrs(def_id: DefId) -> &'tcx [ty::DeducedParamAttrs] { + query deduced_param_attrs(def_id: DefId) -> &'tcx [DeducedParamAttrs] { desc { |tcx| "deducing parameter attributes for {}", tcx.def_path_str(def_id) } separate_provide_extern } diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index 546791135ba61..e8952d0492d1e 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -799,7 +799,7 @@ impl_ref_decoder! {<'tcx> rustc_span::def_id::DefId, rustc_span::def_id::LocalDefId, (rustc_middle::middle::exported_symbols::ExportedSymbol<'tcx>, rustc_middle::middle::exported_symbols::SymbolExportInfo), - ty::DeducedParamAttrs, + rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs, } //- ENCODING ------------------------------------------------------------------- diff --git a/compiler/rustc_middle/src/ty/codec.rs b/compiler/rustc_middle/src/ty/codec.rs index 3f37595d0eef0..3d6320be3af81 100644 --- a/compiler/rustc_middle/src/ty/codec.rs +++ b/compiler/rustc_middle/src/ty/codec.rs @@ -577,7 +577,7 @@ impl_arena_copy_decoder! {<'tcx> rustc_span::def_id::DefId, rustc_span::def_id::LocalDefId, (rustc_middle::middle::exported_symbols::ExportedSymbol<'tcx>, rustc_middle::middle::exported_symbols::SymbolExportInfo), - ty::DeducedParamAttrs, + rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs, } #[macro_export] diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 3c5c21a7a89c2..9e84e7080dfee 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -41,7 +41,6 @@ use rustc_hir::lang_items::LangItem; use rustc_hir::limit::Limit; use rustc_hir::{self as hir, Attribute, HirId, Node, TraitCandidate, find_attr}; use rustc_index::IndexVec; -use rustc_macros::{HashStable, TyDecodable, TyEncodable}; use rustc_query_system::cache::WithDepNode; use rustc_query_system::dep_graph::DepNodeIndex; use rustc_query_system::ich::StableHashingContext; @@ -3567,21 +3566,6 @@ impl<'tcx> TyCtxt<'tcx> { } } -/// Parameter attributes that can only be determined by examining the body of a function instead -/// of just its signature. -/// -/// These can be useful for optimization purposes when a function is directly called. We compute -/// them and store them into the crate metadata so that downstream crates can make use of them. -/// -/// Right now, we only have `read_only`, but `no_capture` and `no_alias` might be useful in the -/// future. -#[derive(Clone, Copy, PartialEq, Debug, Default, TyDecodable, TyEncodable, HashStable)] -pub struct DeducedParamAttrs { - /// The parameter is marked immutable in the function and contains no `UnsafeCell` (i.e. its - /// type is freeze). - pub read_only: bool, -} - pub fn provide(providers: &mut Providers) { providers.is_panic_runtime = |tcx, LocalCrate| contains_name(tcx.hir_krate_attrs(), sym::panic_runtime); diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index ce4de6b95e0bb..388fb89a0870f 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -78,8 +78,7 @@ pub use self::consts::{ ExprKind, ScalarInt, UnevaluatedConst, ValTree, ValTreeKind, Value, }; pub use self::context::{ - CtxtInterners, CurrentGcx, DeducedParamAttrs, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt, - TyCtxtFeed, tls, + CtxtInterners, CurrentGcx, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt, TyCtxtFeed, tls, }; pub use self::fold::*; pub use self::instance::{Instance, InstanceKind, ReifyReason, UnusedGenericParams}; diff --git a/compiler/rustc_mir_transform/src/deduce_param_attrs.rs b/compiler/rustc_mir_transform/src/deduce_param_attrs.rs index a0db8bdb7ed88..2ba1aa05024f3 100644 --- a/compiler/rustc_mir_transform/src/deduce_param_attrs.rs +++ b/compiler/rustc_mir_transform/src/deduce_param_attrs.rs @@ -6,55 +6,66 @@ //! dependent crates can use them. use rustc_hir::def_id::LocalDefId; -use rustc_index::bit_set::DenseBitSet; -use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; -use rustc_middle::mir::{Body, Location, Operand, Place, RETURN_PLACE, Terminator, TerminatorKind}; -use rustc_middle::ty::{self, DeducedParamAttrs, Ty, TyCtxt}; +use rustc_index::IndexVec; +use rustc_middle::middle::deduced_param_attrs::{DeducedParamAttrs, DeducedReadOnlyParam}; +use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; +use rustc_middle::mir::*; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::config::OptLevel; /// A visitor that determines which arguments have been mutated. We can't use the mutability field /// on LocalDecl for this because it has no meaning post-optimization. struct DeduceReadOnly { /// Each bit is indexed by argument number, starting at zero (so 0 corresponds to local decl - /// 1). The bit is true if the argument may have been mutated or false if we know it hasn't + /// 1). The bit is false if the argument may have been mutated or true if we know it hasn't /// been up to the point we're at. - mutable_args: DenseBitSet, + read_only: IndexVec, } impl DeduceReadOnly { /// Returns a new DeduceReadOnly instance. fn new(arg_count: usize) -> Self { - Self { mutable_args: DenseBitSet::new_empty(arg_count) } + Self { read_only: IndexVec::from_elem_n(DeducedReadOnlyParam::empty(), arg_count) } + } + + /// Returns whether the given local is a parameter and its index. + fn as_param(&self, local: Local) -> Option { + // Locals and parameters are shifted by `RETURN_PLACE`. + let param_index = local.as_usize().checked_sub(1)?; + if param_index < self.read_only.len() { Some(param_index) } else { None } } } impl<'tcx> Visitor<'tcx> for DeduceReadOnly { fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) { // We're only interested in arguments. - if place.local == RETURN_PLACE || place.local.index() > self.mutable_args.domain_size() { - return; - } - - let mark_as_mutable = match context { - PlaceContext::MutatingUse(..) => { - // This is a mutation, so mark it as such. - true + let Some(param_index) = self.as_param(place.local) else { return }; + + match context { + // Not mutating, so it's fine. + PlaceContext::NonUse(..) => {} + // Dereference is not a mutation. + _ if place.is_indirect_first_projection() => {} + // This is a `Drop`. It could disappear at monomorphization, so mark it specially. + PlaceContext::MutatingUse(MutatingUseContext::Drop) + // Projection changes the place's type, so `needs_drop(local.ty)` is not + // `needs_drop(place.ty)`. + if place.projection.is_empty() => { + self.read_only[param_index] |= DeducedReadOnlyParam::IF_NO_DROP; } - PlaceContext::NonMutatingUse(NonMutatingUseContext::RawBorrow) => { - // Whether mutating though a `&raw const` is allowed is still undecided, so we - // disable any sketchy `readonly` optimizations for now. But we only need to do - // this if the pointer would point into the argument. IOW: for indirect places, - // like `&raw (*local).field`, this surely cannot mutate `local`. - !place.is_indirect() + // This is a mutation, so mark it as such. + PlaceContext::MutatingUse(..) + // Whether mutating though a `&raw const` is allowed is still undecided, so we + // disable any sketchy `readonly` optimizations for now. + | PlaceContext::NonMutatingUse(NonMutatingUseContext::RawBorrow) => { + self.read_only[param_index] |= DeducedReadOnlyParam::MUTATED; } - PlaceContext::NonMutatingUse(..) | PlaceContext::NonUse(..) => { - // Not mutating, so it's fine. - false + // Not mutating if the parameter is `Freeze`. + PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) => { + self.read_only[param_index] |= DeducedReadOnlyParam::IF_FREEZE; } - }; - - if mark_as_mutable { - self.mutable_args.insert(place.local.index() - 1); + // Not mutating, so it's fine. + PlaceContext::NonMutatingUse(..) => {} } } @@ -82,16 +93,12 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly { // from. if let TerminatorKind::Call { ref args, .. } = terminator.kind { for arg in args { - if let Operand::Move(place) = arg.node { - let local = place.local; - if place.is_indirect() - || local == RETURN_PLACE - || local.index() > self.mutable_args.domain_size() - { - continue; - } - - self.mutable_args.insert(local.index() - 1); + if let Operand::Move(place) = arg.node + // We're only interested in arguments. + && let Some(param_index) = self.as_param(place.local) + && !place.is_indirect_first_projection() + { + self.read_only[param_index] |= DeducedReadOnlyParam::MUTATED; } } }; @@ -121,6 +128,7 @@ fn type_will_always_be_passed_directly(ty: Ty<'_>) -> bool { /// body of the function instead of just the signature. These can be useful for optimization /// purposes on a best-effort basis. We compute them here and store them into the crate metadata so /// dependent crates can use them. +#[tracing::instrument(level = "trace", skip(tcx), ret)] pub(super) fn deduced_param_attrs<'tcx>( tcx: TyCtxt<'tcx>, def_id: LocalDefId, @@ -160,36 +168,19 @@ pub(super) fn deduced_param_attrs<'tcx>( let body: &Body<'tcx> = tcx.optimized_mir(def_id); let mut deduce_read_only = DeduceReadOnly::new(body.arg_count); deduce_read_only.visit_body(body); + tracing::trace!(?deduce_read_only.read_only); - // Set the `readonly` attribute for every argument that we concluded is immutable and that - // contains no UnsafeCells. - // - // FIXME: This is overly conservative around generic parameters: `is_freeze()` will always - // return false for them. For a description of alternatives that could do a better job here, - // see [1]. - // - // [1]: https://github.com/rust-lang/rust/pull/103172#discussion_r999139997 - let typing_env = body.typing_env(tcx); - let mut deduced_param_attrs = tcx.arena.alloc_from_iter( - body.local_decls.iter().skip(1).take(body.arg_count).enumerate().map( - |(arg_index, local_decl)| DeducedParamAttrs { - read_only: !deduce_read_only.mutable_args.contains(arg_index) - // We must normalize here to reveal opaques and normalize - // their generic parameters, otherwise we'll see exponential - // blow-up in compile times: #113372 - && tcx - .normalize_erasing_regions(typing_env, local_decl.ty) - .is_freeze(tcx, typing_env), - }, - ), + let mut deduced_param_attrs: &[_] = tcx.arena.alloc_from_iter( + deduce_read_only.read_only.into_iter().map(|read_only| DeducedParamAttrs { read_only }), ); // Trailing parameters past the size of the `deduced_param_attrs` array are assumed to have the // default set of attributes, so we don't have to store them explicitly. Pop them off to save a // few bytes in metadata. - while deduced_param_attrs.last() == Some(&DeducedParamAttrs::default()) { - let last_index = deduced_param_attrs.len() - 1; - deduced_param_attrs = &mut deduced_param_attrs[0..last_index]; + while let Some((last, rest)) = deduced_param_attrs.split_last() + && last.is_default() + { + deduced_param_attrs = rest; } deduced_param_attrs diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index e4ed084b073b6..ed5289c6850e8 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -640,10 +640,10 @@ fn fn_abi_adjust_for_abi<'tcx>( // we can't deduce any parameters for, so make sure the argument index is in // bounds. if let Some(deduced_param_attrs) = deduced_param_attrs.get(arg_idx) - && deduced_param_attrs.read_only + && deduced_param_attrs.read_only(tcx, cx.typing_env, arg.layout.ty) { - attrs.regular.insert(ArgAttribute::ReadOnly); debug!("added deduced read-only attribute"); + attrs.regular.insert(ArgAttribute::ReadOnly); } } } diff --git a/tests/codegen-llvm/deduced-param-attrs.rs b/tests/codegen-llvm/deduced-param-attrs.rs index 34504c80fad1b..f99615cbe6d42 100644 --- a/tests/codegen-llvm/deduced-param-attrs.rs +++ b/tests/codegen-llvm/deduced-param-attrs.rs @@ -1,4 +1,4 @@ -//@ compile-flags: -Copt-level=3 +//@ compile-flags: -Copt-level=3 -Cno-prepopulate-passes #![crate_type = "lib"] #![allow(internal_features)] @@ -28,7 +28,9 @@ pub fn use_big_struct_immutably(big_struct: BigStruct) { // The by-value parameter for this big struct can't be marked readonly, because we mutate it. // -// CHECK-NOT: @use_big_struct_mutably({{.*}} readonly {{.*}} %big_struct) +// CHECK: @use_big_struct_mutably( +// CHECK-NOT: readonly +// CHECK-SAME: %big_struct) #[no_mangle] pub fn use_big_struct_mutably(mut big_struct: BigStruct) { big_struct.blah[987] = 654; @@ -38,7 +40,9 @@ pub fn use_big_struct_mutably(mut big_struct: BigStruct) { // The by-value parameter for this big struct can't be marked readonly, because it contains // UnsafeCell. // -// CHECK-NOT: @use_big_cell_container({{.*}} readonly {{.*}} %big_cell_container) +// CHECK: @use_big_cell_container( +// CHECK-NOT: readonly +// CHECK-SAME: %big_cell_container) #[no_mangle] pub fn use_big_cell_container(big_cell_container: BigCellContainer) { hint::black_box(&big_cell_container); @@ -47,14 +51,31 @@ pub fn use_big_cell_container(big_cell_container: BigCellContainer) { // Make sure that we don't mistakenly mark a big struct as `readonly` when passed through a generic // type parameter if it contains UnsafeCell. // -// CHECK-NOT: @use_something({{.*}} readonly {{.*}} %something) +// CHECK: @use_something( +// CHECK-NOT: readonly +// CHECK-SAME: %something) #[no_mangle] #[inline(never)] pub fn use_something(something: T) { hint::black_box(&something); } +// Make sure that we still mark a big `Freeze` struct as `readonly` when passed through a generic +// type parameter. +// +// CHECK: @use_something_freeze( +// CHECK-SAME: readonly +// CHECK-SAME: %x) +#[no_mangle] +#[inline(never)] +pub fn use_something_freeze(x: T) {} + #[no_mangle] pub fn forward_big_cell_container(big_cell_container: BigCellContainer) { use_something(big_cell_container) } + +#[no_mangle] +pub fn forward_big_container(big_struct: BigStruct) { + use_something_freeze(big_struct) +}