diff options
Diffstat (limited to 'compiler')
21 files changed, 418 insertions, 411 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 0752c718c70..c5d1ebdfe7c 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,159 +1,38 @@ use rustc_data_structures::captures::Captures; -use rustc_data_structures::fx::FxIndexSet; -use rustc_index::bit_set::BitSet; -use rustc_middle::mir::CoverageIdsInfo; use rustc_middle::mir::coverage::{ - CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op, + CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, SourceRegion, }; -use rustc_middle::ty::Instance; -use tracing::debug; use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; -/// Holds all of the coverage mapping data associated with a function instance, -/// collected during traversal of `Coverage` statements in the function's MIR. -#[derive(Debug)] -pub(crate) struct FunctionCoverageCollector<'tcx> { - /// Coverage info that was attached to this function by the instrumentor. - function_coverage_info: &'tcx FunctionCoverageInfo, - ids_info: &'tcx CoverageIdsInfo, - is_used: bool, +pub(crate) struct FunctionCoverage<'tcx> { + pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, + /// If `None`, the corresponding function is unused. + ids_info: Option<&'tcx CoverageIdsInfo>, } -impl<'tcx> FunctionCoverageCollector<'tcx> { - /// Creates a new set of coverage data for a used (called) function. - pub(crate) fn new( - instance: Instance<'tcx>, - function_coverage_info: &'tcx FunctionCoverageInfo, - ids_info: &'tcx CoverageIdsInfo, - ) -> Self { - Self::create(instance, function_coverage_info, ids_info, true) - } - - /// Creates a new set of coverage data for an unused (never called) function. - pub(crate) fn unused( - instance: Instance<'tcx>, - function_coverage_info: &'tcx FunctionCoverageInfo, - ids_info: &'tcx CoverageIdsInfo, - ) -> Self { - Self::create(instance, function_coverage_info, ids_info, false) - } - - fn create( - instance: Instance<'tcx>, +impl<'tcx> FunctionCoverage<'tcx> { + pub(crate) fn new_used( function_coverage_info: &'tcx FunctionCoverageInfo, ids_info: &'tcx CoverageIdsInfo, - is_used: bool, ) -> Self { - let num_counters = function_coverage_info.num_counters; - let num_expressions = function_coverage_info.expressions.len(); - debug!( - "FunctionCoverage::create(instance={instance:?}) has \ - num_counters={num_counters}, num_expressions={num_expressions}, is_used={is_used}" - ); - - Self { function_coverage_info, ids_info, is_used } - } - - /// Identify expressions that will always have a value of zero, and note - /// their IDs in [`ZeroExpressions`]. Mappings that refer to a zero expression - /// can instead become mappings to a constant zero value. - /// - /// This method mainly exists to preserve the simplifications that were - /// already being performed by the Rust-side expression renumbering, so that - /// the resulting coverage mappings don't get worse. - fn identify_zero_expressions(&self) -> ZeroExpressions { - // The set of expressions that either were optimized out entirely, or - // have zero as both of their operands, and will therefore always have - // a value of zero. Other expressions that refer to these as operands - // can have those operands replaced with `CovTerm::Zero`. - let mut zero_expressions = ZeroExpressions::default(); - - // Simplify a copy of each expression based on lower-numbered expressions, - // and then update the set of always-zero expressions if necessary. - // (By construction, expressions can only refer to other expressions - // that have lower IDs, so one pass is sufficient.) - for (id, expression) in self.function_coverage_info.expressions.iter_enumerated() { - if !self.is_used || !self.ids_info.expressions_seen.contains(id) { - // If an expression was not seen, it must have been optimized away, - // so any operand that refers to it can be replaced with zero. - zero_expressions.insert(id); - continue; - } - - // We don't need to simplify the actual expression data in the - // expressions list; we can just simplify a temporary copy and then - // use that to update the set of always-zero expressions. - let Expression { mut lhs, op, mut rhs } = *expression; - - // If an expression has an operand that is also an expression, the - // operand's ID must be strictly lower. This is what lets us find - // all zero expressions in one pass. - let assert_operand_expression_is_lower = |operand_id: ExpressionId| { - assert!( - operand_id < id, - "Operand {operand_id:?} should be less than {id:?} in {expression:?}", - ) - }; - - // If an operand refers to a counter or expression that is always - // zero, then that operand can be replaced with `CovTerm::Zero`. - let maybe_set_operand_to_zero = |operand: &mut CovTerm| { - if let CovTerm::Expression(id) = *operand { - assert_operand_expression_is_lower(id); - } - - if is_zero_term(&self.ids_info.counters_seen, &zero_expressions, *operand) { - *operand = CovTerm::Zero; - } - }; - maybe_set_operand_to_zero(&mut lhs); - maybe_set_operand_to_zero(&mut rhs); - - // Coverage counter values cannot be negative, so if an expression - // involves subtraction from zero, assume that its RHS must also be zero. - // (Do this after simplifications that could set the LHS to zero.) - if lhs == CovTerm::Zero && op == Op::Subtract { - rhs = CovTerm::Zero; - } - - // After the above simplifications, if both operands are zero, then - // we know that this expression is always zero too. - if lhs == CovTerm::Zero && rhs == CovTerm::Zero { - zero_expressions.insert(id); - } - } - - zero_expressions + Self { function_coverage_info, ids_info: Some(ids_info) } } - pub(crate) fn into_finished(self) -> FunctionCoverage<'tcx> { - let zero_expressions = self.identify_zero_expressions(); - let FunctionCoverageCollector { function_coverage_info, ids_info, is_used, .. } = self; - - FunctionCoverage { function_coverage_info, ids_info, is_used, zero_expressions } + pub(crate) fn new_unused(function_coverage_info: &'tcx FunctionCoverageInfo) -> Self { + Self { function_coverage_info, ids_info: None } } -} - -pub(crate) struct FunctionCoverage<'tcx> { - pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, - ids_info: &'tcx CoverageIdsInfo, - is_used: bool, - - zero_expressions: ZeroExpressions, -} -impl<'tcx> FunctionCoverage<'tcx> { /// Returns true for a used (called) function, and false for an unused function. pub(crate) fn is_used(&self) -> bool { - self.is_used + self.ids_info.is_some() } /// Return the source hash, generated from the HIR node structure, and used to indicate whether /// or not the source code structure changed between different compilations. pub(crate) fn source_hash(&self) -> u64 { - if self.is_used { self.function_coverage_info.function_source_hash } else { 0 } + if self.is_used() { self.function_coverage_info.function_source_hash } else { 0 } } /// Convert this function's coverage expression data into a form that can be @@ -196,37 +75,10 @@ impl<'tcx> FunctionCoverage<'tcx> { } fn is_zero_term(&self, term: CovTerm) -> bool { - !self.is_used || is_zero_term(&self.ids_info.counters_seen, &self.zero_expressions, term) - } -} - -/// Set of expression IDs that are known to always evaluate to zero. -/// Any mapping or expression operand that refers to these expressions can have -/// that reference replaced with a constant zero value. -#[derive(Default)] -struct ZeroExpressions(FxIndexSet<ExpressionId>); - -impl ZeroExpressions { - fn insert(&mut self, id: ExpressionId) { - self.0.insert(id); - } - - fn contains(&self, id: ExpressionId) -> bool { - self.0.contains(&id) - } -} - -/// Returns `true` if the given term is known to have a value of zero, taking -/// into account knowledge of which counters are unused and which expressions -/// are always zero. -fn is_zero_term( - counters_seen: &BitSet<CounterId>, - zero_expressions: &ZeroExpressions, - term: CovTerm, -) -> bool { - match term { - CovTerm::Zero => true, - CovTerm::Counter(id) => !counters_seen.contains(id), - CovTerm::Expression(id) => zero_expressions.contains(id), + match self.ids_info { + Some(ids_info) => ids_info.is_zero_term(term), + // This function is unused, so all coverage counters/expressions are zero. + None => true, + } } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 8c24579fa7c..a6c3caf9e2b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -20,7 +20,7 @@ use rustc_target::spec::HasTargetSpec; use tracing::debug; use crate::common::CodegenCx; -use crate::coverageinfo::map_data::{FunctionCoverage, FunctionCoverageCollector}; +use crate::coverageinfo::map_data::FunctionCoverage; use crate::coverageinfo::{ffi, llvm_cov}; use crate::llvm; @@ -63,16 +63,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { None => return, }; if function_coverage_map.is_empty() { - // This module has no functions with coverage instrumentation + // This CGU has no functions with coverage instrumentation. return; } - let function_coverage_entries = function_coverage_map - .into_iter() - .map(|(instance, function_coverage)| (instance, function_coverage.into_finished())) - .collect::<Vec<_>>(); - - let all_file_names = function_coverage_entries + let all_file_names = function_coverage_map .iter() .map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span) .map(|span| span_file_name(tcx, span)); @@ -92,7 +87,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { let mut unused_function_names = Vec::new(); // Encode coverage mappings and generate function records - for (instance, function_coverage) in function_coverage_entries { + for (instance, function_coverage) in function_coverage_map { debug!("Generate function coverage for {}, {:?}", cx.codegen_unit.name(), instance); let mangled_function_name = tcx.symbol_name(instance).name; @@ -536,11 +531,6 @@ fn add_unused_function_coverage<'tcx>( ); // An unused function's mappings will all be rewritten to map to zero. - let function_coverage = FunctionCoverageCollector::unused( - instance, - function_coverage_info, - tcx.coverage_ids_info(instance.def), - ); - + let function_coverage = FunctionCoverage::new_unused(function_coverage_info); cx.coverage_cx().function_coverage_map.borrow_mut().insert(instance, function_coverage); } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index c2fcb33f98b..82b6677e203 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -13,7 +13,7 @@ use tracing::{debug, instrument}; use crate::builder::Builder; use crate::common::CodegenCx; -use crate::coverageinfo::map_data::FunctionCoverageCollector; +use crate::coverageinfo::map_data::FunctionCoverage; use crate::llvm; pub(crate) mod ffi; @@ -24,8 +24,7 @@ mod mapgen; /// Extra per-CGU context/state needed for coverage instrumentation. pub(crate) struct CguCoverageContext<'ll, 'tcx> { /// Coverage data for each instrumented function identified by DefId. - pub(crate) function_coverage_map: - RefCell<FxIndexMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>>>, + pub(crate) function_coverage_map: RefCell<FxIndexMap<Instance<'tcx>, FunctionCoverage<'tcx>>>, pub(crate) pgo_func_name_var_map: RefCell<FxHashMap<Instance<'tcx>, &'ll llvm::Value>>, pub(crate) mcdc_condition_bitmap_map: RefCell<FxHashMap<Instance<'tcx>, Vec<&'ll llvm::Value>>>, @@ -42,9 +41,7 @@ impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> { } } - fn take_function_coverage_map( - &self, - ) -> FxIndexMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>> { + fn take_function_coverage_map(&self) -> FxIndexMap<Instance<'tcx>, FunctionCoverage<'tcx>> { self.function_coverage_map.replace(FxIndexMap::default()) } @@ -161,8 +158,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { // This includes functions that were not partitioned into this CGU, // but were MIR-inlined into one of this CGU's functions. coverage_cx.function_coverage_map.borrow_mut().entry(instance).or_insert_with(|| { - FunctionCoverageCollector::new( - instance, + FunctionCoverage::new_used( function_coverage_info, bx.tcx.coverage_ids_info(instance.def), ) diff --git a/compiler/rustc_codegen_ssa/Cargo.toml b/compiler/rustc_codegen_ssa/Cargo.toml index b898cfec796..c81e36dfc8d 100644 --- a/compiler/rustc_codegen_ssa/Cargo.toml +++ b/compiler/rustc_codegen_ssa/Cargo.toml @@ -17,6 +17,7 @@ regex = "1.4" rustc_abi = { path = "../rustc_abi" } rustc_arena = { path = "../rustc_arena" } rustc_ast = { path = "../rustc_ast" } +rustc_ast_pretty = { path = "../rustc_ast_pretty" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } @@ -42,7 +43,7 @@ tempfile = "3.2" thin-vec = "0.2.12" thorin-dwp = "0.8" tracing = "0.1" -wasm-encoder = "0.216.0" +wasm-encoder = "0.219" # tidy-alphabetical-end [target.'cfg(unix)'.dependencies] diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index 62db3d5a98c..56188714b44 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -201,6 +201,11 @@ codegen_ssa_missing_memory_ordering = Atomic intrinsic missing memory ordering codegen_ssa_missing_query_depgraph = found CGU-reuse attribute but `-Zquery-dep-graph` was not specified +codegen_ssa_mixed_export_name_and_no_mangle = `{$no_mangle_attr}` attribute may not be used in combination with `#[export_name]` + .label = `{$no_mangle_attr}` is ignored + .note = `#[export_name]` takes precedence + .suggestion = remove the `{$no_mangle_attr}` attribute + codegen_ssa_msvc_missing_linker = the msvc targets depend on the msvc linker but `link.exe` was not found codegen_ssa_multiple_external_func_decl = multiple declarations of external function `{$function}` from library `{$library_name}` have different calling conventions diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index a5acd8170ab..11744eabab0 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -3,11 +3,10 @@ use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr, list_contains_nam use rustc_data_structures::fx::FxHashMap; use rustc_errors::codes::*; use rustc_errors::{DiagMessage, SubdiagMessage, struct_span_code_err}; -use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId}; use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS; -use rustc_hir::{LangItem, lang_items}; +use rustc_hir::{self as hir, HirId, LangItem, lang_items}; use rustc_middle::middle::codegen_fn_attrs::{ CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, }; @@ -78,6 +77,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { let mut inline_span = None; let mut link_ordinal_span = None; let mut no_sanitize_span = None; + let mut mixed_export_name_no_mangle_lint_state = MixedExportNameAndNoMangleState::default(); for attr in attrs.iter() { // In some cases, attribute are only valid on functions, but it's the `check_attr` @@ -116,7 +116,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { sym::naked => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED, sym::no_mangle => { if tcx.opt_item_name(did.to_def_id()).is_some() { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE; + mixed_export_name_no_mangle_lint_state.track_no_mangle( + attr.span, + tcx.local_def_id_to_hir_id(did), + attr, + ); } else { tcx.dcx() .struct_span_err( @@ -240,6 +245,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { .emit(); } codegen_fn_attrs.export_name = Some(s); + mixed_export_name_no_mangle_lint_state.track_export_name(attr.span); } } sym::target_feature => { @@ -513,6 +519,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } } + mixed_export_name_no_mangle_lint_state.lint_if_mixed(tcx); + codegen_fn_attrs.inline = attrs.iter().fold(InlineAttr::None, |ia, attr| { if !attr.has_name(sym::inline) { return ia; @@ -779,6 +787,49 @@ fn check_link_name_xor_ordinal( } } +#[derive(Default)] +struct MixedExportNameAndNoMangleState<'a> { + export_name: Option<Span>, + hir_id: Option<HirId>, + no_mangle: Option<Span>, + no_mangle_attr: Option<&'a ast::Attribute>, +} + +impl<'a> MixedExportNameAndNoMangleState<'a> { + fn track_export_name(&mut self, span: Span) { + self.export_name = Some(span); + } + + fn track_no_mangle(&mut self, span: Span, hir_id: HirId, attr_name: &'a ast::Attribute) { + self.no_mangle = Some(span); + self.hir_id = Some(hir_id); + self.no_mangle_attr = Some(attr_name); + } + + /// Emit diagnostics if the lint condition is met. + fn lint_if_mixed(self, tcx: TyCtxt<'_>) { + if let Self { + export_name: Some(export_name), + no_mangle: Some(no_mangle), + hir_id: Some(hir_id), + no_mangle_attr: Some(no_mangle_attr), + } = self + { + tcx.emit_node_span_lint( + lint::builtin::UNUSED_ATTRIBUTES, + hir_id, + no_mangle, + errors::MixedExportNameAndNoMangle { + no_mangle, + no_mangle_attr: rustc_ast_pretty::pprust::attribute_to_string(no_mangle_attr), + export_name, + removal_span: no_mangle, + }, + ); + } + } +} + pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { codegen_fn_attrs, should_inherit_track_caller, ..*providers }; } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index f93cb52ea3e..00f8654e670 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -10,7 +10,7 @@ use rustc_errors::codes::*; use rustc_errors::{ Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, IntoDiagArg, Level, }; -use rustc_macros::{Diagnostic, Subdiagnostic}; +use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::Ty; use rustc_middle::ty::layout::LayoutError; use rustc_span::{Span, Symbol}; @@ -1114,3 +1114,15 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for TargetFeatureDisableOrEnable<'_ #[derive(Diagnostic)] #[diag(codegen_ssa_aix_strip_not_used)] pub(crate) struct AixStripNotUsed; + +#[derive(LintDiagnostic)] +#[diag(codegen_ssa_mixed_export_name_and_no_mangle)] +pub(crate) struct MixedExportNameAndNoMangle { + #[label] + pub no_mangle: Span, + pub no_mangle_attr: String, + #[note] + pub export_name: Span, + #[suggestion(style = "verbose", code = "", applicability = "machine-applicable")] + pub removal_span: Span, +} diff --git a/compiler/rustc_const_eval/src/const_eval/error.rs b/compiler/rustc_const_eval/src/const_eval/error.rs index 3cb77d1dcb5..fee7287951d 100644 --- a/compiler/rustc_const_eval/src/const_eval/error.rs +++ b/compiler/rustc_const_eval/src/const_eval/error.rs @@ -170,7 +170,7 @@ where let reported = if allowed_in_infallible { ReportedErrorInfo::allowed_in_infallible(g) } else { - ReportedErrorInfo::from(g) + ReportedErrorInfo::const_eval_error(g) }; ErrorHandled::Reported(reported, span) } diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 647d880e2bf..ce8eceebdf8 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -3,13 +3,13 @@ use std::sync::atomic::Ordering::Relaxed; use either::{Left, Right}; use rustc_abi::{self as abi, BackendRepr}; use rustc_hir::def::DefKind; -use rustc_middle::bug; -use rustc_middle::mir::interpret::{AllocId, ErrorHandled, InterpErrorInfo}; +use rustc_middle::mir::interpret::{AllocId, ErrorHandled, InterpErrorInfo, ReportedErrorInfo}; use rustc_middle::mir::{self, ConstAlloc, ConstValue}; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::layout::{HasTypingEnv, LayoutOf}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::{bug, throw_inval}; use rustc_span::def_id::LocalDefId; use rustc_span::{DUMMY_SP, Span}; use tracing::{debug, instrument, trace}; @@ -93,18 +93,18 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>( match intern_result { Ok(()) => {} Err(InternResult::FoundDanglingPointer) => { - return Err(ecx - .tcx - .dcx() - .emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })) - .into(); + throw_inval!(AlreadyReported(ReportedErrorInfo::non_const_eval_error( + ecx.tcx + .dcx() + .emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }), + ))); } Err(InternResult::FoundBadMutablePointer) => { - return Err(ecx - .tcx - .dcx() - .emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind })) - .into(); + throw_inval!(AlreadyReported(ReportedErrorInfo::non_const_eval_error( + ecx.tcx + .dcx() + .emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind }), + ))); } } diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 11e0fac51d8..56325eaa1be 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -8,6 +8,7 @@ use rustc_data_structures::fx::{FxHashMap, FxIndexMap, IndexEntry}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::{self as hir, CRATE_HIR_ID, LangItem}; use rustc_middle::mir::AssertMessage; +use rustc_middle::mir::interpret::ReportedErrorInfo; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout}; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -563,7 +564,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { .tcx .dcx() .span_delayed_bug(span, "The deny lint should have already errored"); - throw_inval!(AlreadyReported(guard.into())); + throw_inval!(AlreadyReported(ReportedErrorInfo::allowed_in_infallible(guard))); } } else if new_steps > start && new_steps.is_power_of_two() { // Only report after a certain number of terminators have been evaluated and the diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index 515028e6826..6f51b09323d 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -1,6 +1,6 @@ use rustc_abi::{BackendRepr, VariantIdx}; use rustc_data_structures::stack::ensure_sufficient_stack; -use rustc_middle::mir::interpret::{EvalToValTreeResult, GlobalId}; +use rustc_middle::mir::interpret::{EvalToValTreeResult, GlobalId, ReportedErrorInfo}; use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout}; use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt}; use rustc_middle::{bug, mir}; @@ -261,7 +261,7 @@ pub(crate) fn eval_to_valtree<'tcx>( ValTreeCreationError::NodesOverflow => { let handled = tcx.dcx().emit_err(MaxNumNodesInConstErr { span, global_const_id }); - Err(handled.into()) + Err(ReportedErrorInfo::allowed_in_infallible(handled).into()) } ValTreeCreationError::NonSupportedType(ty) => Ok(Err(ty)), } diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 241be5e175c..95a72d3cbc1 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -268,7 +268,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { }; // do not continue if typeck errors occurred (can only occur in local crate) if let Some(err) = body.tainted_by_errors { - throw_inval!(AlreadyReported(ReportedErrorInfo::from(err))); + throw_inval!(AlreadyReported(ReportedErrorInfo::non_const_eval_error(err))); } interp_ok(body) } @@ -317,7 +317,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { Ok(None) => throw_inval!(TooGeneric), // FIXME(eddyb) this could be a bit more specific than `AlreadyReported`. - Err(error_reported) => throw_inval!(AlreadyReported(error_reported.into())), + Err(error_guaranteed) => throw_inval!(AlreadyReported( + ReportedErrorInfo::non_const_eval_error(error_guaranteed) + )), } } diff --git a/compiler/rustc_middle/src/mir/consts.rs b/compiler/rustc_middle/src/mir/consts.rs index 7983329b0f7..52009422d98 100644 --- a/compiler/rustc_middle/src/mir/consts.rs +++ b/compiler/rustc_middle/src/mir/consts.rs @@ -8,6 +8,7 @@ use rustc_session::config::RemapPathScopeComponents; use rustc_span::{DUMMY_SP, Span}; use rustc_type_ir::visit::TypeVisitableExt; +use super::interpret::ReportedErrorInfo; use crate::mir::interpret::{AllocId, ConstAllocation, ErrorHandled, Scalar, alloc_range}; use crate::mir::{Promoted, pretty_print_const_value}; use crate::ty::print::{pretty_print_const, with_no_trimmed_paths}; @@ -331,7 +332,10 @@ impl<'tcx> Const<'tcx> { ConstKind::Expr(_) => { bug!("Normalization of `ty::ConstKind::Expr` is unimplemented") } - _ => Err(tcx.dcx().delayed_bug("Unevaluated `ty::Const` in MIR body").into()), + _ => Err(ReportedErrorInfo::non_const_eval_error( + tcx.dcx().delayed_bug("Unevaluated `ty::Const` in MIR body"), + ) + .into()), } } Const::Unevaluated(uneval, _) => { diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index b7410ca5f18..962176290df 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -3,6 +3,7 @@ use std::fmt::{self, Debug, Formatter}; use rustc_index::IndexVec; +use rustc_index::bit_set::BitSet; use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable}; use rustc_span::Span; @@ -310,3 +311,41 @@ pub struct MCDCDecisionSpan { pub decision_depth: u16, pub num_conditions: usize, } + +/// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass +/// (for compiler option `-Cinstrument-coverage`), after MIR optimizations +/// have had a chance to potentially remove some of them. +/// +/// Used by the `coverage_ids_info` query. +#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)] +pub struct CoverageIdsInfo { + pub counters_seen: BitSet<CounterId>, + pub zero_expressions: BitSet<ExpressionId>, +} + +impl CoverageIdsInfo { + /// Coverage codegen needs to know how many coverage counters are ever + /// incremented within a function, so that it can set the `num-counters` + /// argument of the `llvm.instrprof.increment` intrinsic. + /// + /// This may be less than the highest counter ID emitted by the + /// InstrumentCoverage MIR pass, if the highest-numbered counter increments + /// were removed by MIR optimizations. + pub fn num_counters_after_mir_opts(&self) -> u32 { + // FIXME(Zalathar): Currently this treats an unused counter as "used" + // if its ID is less than that of the highest counter that really is + // used. Fixing this would require adding a renumbering step somewhere. + self.counters_seen.last_set_in(..).map_or(0, |max| max.as_u32() + 1) + } + + /// Returns `true` if the given term is known to have a value of zero, taking + /// into account knowledge of which counters are unused and which expressions + /// are always zero. + pub fn is_zero_term(&self, term: CovTerm) -> bool { + match term { + CovTerm::Zero => true, + CovTerm::Counter(id) => !self.counters_seen.contains(id), + CovTerm::Expression(id) => self.zero_expressions.contains(id), + } + } +} diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index ad5d678178d..fbada6ec405 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -28,10 +28,10 @@ pub enum ErrorHandled { TooGeneric(Span), } -impl From<ErrorGuaranteed> for ErrorHandled { +impl From<ReportedErrorInfo> for ErrorHandled { #[inline] - fn from(error: ErrorGuaranteed) -> ErrorHandled { - ErrorHandled::Reported(error.into(), DUMMY_SP) + fn from(error: ReportedErrorInfo) -> ErrorHandled { + ErrorHandled::Reported(error, DUMMY_SP) } } @@ -65,6 +65,20 @@ pub struct ReportedErrorInfo { impl ReportedErrorInfo { #[inline] + pub fn const_eval_error(error: ErrorGuaranteed) -> ReportedErrorInfo { + ReportedErrorInfo { allowed_in_infallible: false, error } + } + + /// Use this when the error that led to this is *not* a const-eval error + /// (e.g., a layout or type checking error). + #[inline] + pub fn non_const_eval_error(error: ErrorGuaranteed) -> ReportedErrorInfo { + ReportedErrorInfo { allowed_in_infallible: true, error } + } + + /// Use this when the error that led to this *is* a const-eval error, but + /// we do allow it to occur in infallible constants (e.g., resource exhaustion). + #[inline] pub fn allowed_in_infallible(error: ErrorGuaranteed) -> ReportedErrorInfo { ReportedErrorInfo { allowed_in_infallible: true, error } } @@ -74,13 +88,6 @@ impl ReportedErrorInfo { } } -impl From<ErrorGuaranteed> for ReportedErrorInfo { - #[inline] - fn from(error: ErrorGuaranteed) -> ReportedErrorInfo { - ReportedErrorInfo { allowed_in_infallible: false, error } - } -} - impl Into<ErrorGuaranteed> for ReportedErrorInfo { #[inline] fn into(self) -> ErrorGuaranteed { @@ -180,12 +187,6 @@ fn print_backtrace(backtrace: &Backtrace) { eprintln!("\n\nAn error occurred in the MIR interpreter:\n{backtrace}"); } -impl From<ErrorGuaranteed> for InterpErrorInfo<'_> { - fn from(err: ErrorGuaranteed) -> Self { - InterpErrorKind::InvalidProgram(InvalidProgramInfo::AlreadyReported(err.into())).into() - } -} - impl From<ErrorHandled> for InterpErrorInfo<'_> { fn from(err: ErrorHandled) -> Self { InterpErrorKind::InvalidProgram(match err { diff --git a/compiler/rustc_middle/src/mir/interpret/queries.rs b/compiler/rustc_middle/src/mir/interpret/queries.rs index e540f0194ec..f7f38575bd0 100644 --- a/compiler/rustc_middle/src/mir/interpret/queries.rs +++ b/compiler/rustc_middle/src/mir/interpret/queries.rs @@ -6,6 +6,7 @@ use tracing::{debug, instrument}; use super::{ ErrorHandled, EvalToAllocationRawResult, EvalToConstValueResult, EvalToValTreeResult, GlobalId, + ReportedErrorInfo, }; use crate::mir; use crate::query::TyCtxtEnsure; @@ -81,7 +82,9 @@ impl<'tcx> TyCtxt<'tcx> { // For errors during resolution, we deliberately do not point at the usage site of the constant, // since for these errors the place the constant is used shouldn't matter. Ok(None) => Err(ErrorHandled::TooGeneric(DUMMY_SP)), - Err(err) => Err(ErrorHandled::Reported(err.into(), DUMMY_SP)), + Err(err) => { + Err(ErrorHandled::Reported(ReportedErrorInfo::non_const_eval_error(err), DUMMY_SP)) + } } } @@ -138,7 +141,9 @@ impl<'tcx> TyCtxt<'tcx> { // For errors during resolution, we deliberately do not point at the usage site of the constant, // since for these errors the place the constant is used shouldn't matter. Ok(None) => Err(ErrorHandled::TooGeneric(DUMMY_SP)), - Err(err) => Err(ErrorHandled::Reported(err.into(), DUMMY_SP)), + Err(err) => { + Err(ErrorHandled::Reported(ReportedErrorInfo::non_const_eval_error(err), DUMMY_SP)) + } } } diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 80dfcbf2e69..f690359e012 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -8,7 +8,7 @@ use rustc_abi::{FieldIdx, VariantIdx}; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::ErrorGuaranteed; use rustc_hir::def_id::LocalDefId; -use rustc_index::bit_set::{BitMatrix, BitSet}; +use rustc_index::bit_set::BitMatrix; use rustc_index::{Idx, IndexVec}; use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable}; use rustc_span::Span; @@ -16,7 +16,6 @@ use rustc_span::symbol::Symbol; use smallvec::SmallVec; use super::{ConstValue, SourceInfo}; -use crate::mir; use crate::ty::fold::fold_regions; use crate::ty::{self, CoroutineArgsExt, OpaqueHiddenType, Ty, TyCtxt}; @@ -351,30 +350,3 @@ pub struct DestructuredConstant<'tcx> { pub variant: Option<VariantIdx>, pub fields: &'tcx [(ConstValue<'tcx>, Ty<'tcx>)], } - -/// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass -/// (for compiler option `-Cinstrument-coverage`), after MIR optimizations -/// have had a chance to potentially remove some of them. -/// -/// Used by the `coverage_ids_info` query. -#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)] -pub struct CoverageIdsInfo { - pub counters_seen: BitSet<mir::coverage::CounterId>, - pub expressions_seen: BitSet<mir::coverage::ExpressionId>, -} - -impl CoverageIdsInfo { - /// Coverage codegen needs to know how many coverage counters are ever - /// incremented within a function, so that it can set the `num-counters` - /// argument of the `llvm.instrprof.increment` intrinsic. - /// - /// This may be less than the highest counter ID emitted by the - /// InstrumentCoverage MIR pass, if the highest-numbered counter increments - /// were removed by MIR optimizations. - pub fn num_counters_after_mir_opts(&self) -> u32 { - // FIXME(Zalathar): Currently this treats an unused counter as "used" - // if its ID is less than that of the highest counter that really is - // used. Fixing this would require adding a renumbering step somewhere. - self.counters_seen.last_set_in(..).map_or(0, |max| max.as_u32() + 1) - } -} diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index a3976c3dda1..fc3d690a8a9 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -581,7 +581,7 @@ rustc_queries! { /// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass /// (for compiler option `-Cinstrument-coverage`), after MIR optimizations /// have had a chance to potentially remove some of them. - query coverage_ids_info(key: ty::InstanceKind<'tcx>) -> &'tcx mir::CoverageIdsInfo { + query coverage_ids_info(key: ty::InstanceKind<'tcx>) -> &'tcx mir::coverage::CoverageIdsInfo { desc { |tcx| "retrieving coverage IDs info from MIR for `{}`", tcx.def_path_str(key.def_id()) } arena_cache } diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 46efdd16ee8..9e80f1f1c4a 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -9,7 +9,7 @@ use rustc_index::bit_set::BitSet; use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, Op}; use tracing::{debug, debug_span, instrument}; -use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, TraverseCoverageGraphWithLoops}; +use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, ReadyFirstTraversal}; #[cfg(test)] mod tests; @@ -236,23 +236,12 @@ impl<'a> CountersBuilder<'a> { // Traverse the coverage graph, ensuring that every node that needs a // coverage counter has one. - // - // The traversal tries to ensure that, when a loop is encountered, all - // nodes within the loop are visited before visiting any nodes outside - // the loop. - let mut traversal = TraverseCoverageGraphWithLoops::new(self.graph); - while let Some(bcb) = traversal.next() { + for bcb in ReadyFirstTraversal::new(self.graph) { let _span = debug_span!("traversal", ?bcb).entered(); if self.bcb_needs_counter.contains(bcb) { self.make_node_counter_and_out_edge_counters(bcb); } } - - assert!( - traversal.is_complete(), - "`TraverseCoverageGraphWithLoops` missed some `BasicCoverageBlock`s: {:?}", - traversal.unvisited(), - ); } /// Make sure the given node has a node counter, and then make sure each of diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 092bce1de2c..ad6774fccd6 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -9,7 +9,6 @@ use rustc_data_structures::graph::dominators::Dominators; use rustc_data_structures::graph::{self, DirectedGraph, StartNode}; use rustc_index::IndexVec; use rustc_index::bit_set::BitSet; -use rustc_middle::bug; use rustc_middle::mir::{self, BasicBlock, Terminator, TerminatorKind}; use tracing::debug; @@ -462,138 +461,6 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera CoverageSuccessors { targets, is_yield } } -/// Maintains separate worklists for each loop in the BasicCoverageBlock CFG, plus one for the -/// CoverageGraph outside all loops. This supports traversing the BCB CFG in a way that -/// ensures a loop is completely traversed before processing Blocks after the end of the loop. -#[derive(Debug)] -struct TraversalContext { - /// BCB with one or more incoming loop backedges, indicating which loop - /// this context is for. - /// - /// If `None`, this is the non-loop context for the function as a whole. - loop_header: Option<BasicCoverageBlock>, - - /// Worklist of BCBs to be processed in this context. - worklist: VecDeque<BasicCoverageBlock>, -} - -pub(crate) struct TraverseCoverageGraphWithLoops<'a> { - basic_coverage_blocks: &'a CoverageGraph, - - context_stack: Vec<TraversalContext>, - visited: BitSet<BasicCoverageBlock>, -} - -impl<'a> TraverseCoverageGraphWithLoops<'a> { - pub(crate) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self { - let worklist = VecDeque::from([basic_coverage_blocks.start_node()]); - let context_stack = vec![TraversalContext { loop_header: None, worklist }]; - - // `context_stack` starts with a `TraversalContext` for the main function context (beginning - // with the `start` BasicCoverageBlock of the function). New worklists are pushed to the top - // of the stack as loops are entered, and popped off of the stack when a loop's worklist is - // exhausted. - let visited = BitSet::new_empty(basic_coverage_blocks.num_nodes()); - Self { basic_coverage_blocks, context_stack, visited } - } - - pub(crate) fn next(&mut self) -> Option<BasicCoverageBlock> { - debug!( - "TraverseCoverageGraphWithLoops::next - context_stack: {:?}", - self.context_stack.iter().rev().collect::<Vec<_>>() - ); - - while let Some(context) = self.context_stack.last_mut() { - let Some(bcb) = context.worklist.pop_front() else { - // This stack level is exhausted; pop it and try the next one. - self.context_stack.pop(); - continue; - }; - - if !self.visited.insert(bcb) { - debug!("Already visited: {bcb:?}"); - continue; - } - debug!("Visiting {bcb:?}"); - - if self.basic_coverage_blocks.is_loop_header.contains(bcb) { - debug!("{bcb:?} is a loop header! Start a new TraversalContext..."); - self.context_stack - .push(TraversalContext { loop_header: Some(bcb), worklist: VecDeque::new() }); - } - self.add_successors_to_worklists(bcb); - return Some(bcb); - } - - None - } - - fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) { - let successors = &self.basic_coverage_blocks.successors[bcb]; - debug!("{:?} has {} successors:", bcb, successors.len()); - - for &successor in successors { - if successor == bcb { - debug!( - "{:?} has itself as its own successor. (Note, the compiled code will \ - generate an infinite loop.)", - bcb - ); - // Don't re-add this successor to the worklist. We are already processing it. - // FIXME: This claims to skip just the self-successor, but it actually skips - // all other successors as well. Does that matter? - break; - } - - // Add successors of the current BCB to the appropriate context. Successors that - // stay within a loop are added to the BCBs context worklist. Successors that - // exit the loop (they are not dominated by the loop header) must be reachable - // from other BCBs outside the loop, and they will be added to a different - // worklist. - // - // Branching blocks (with more than one successor) must be processed before - // blocks with only one successor, to prevent unnecessarily complicating - // `Expression`s by creating a Counter in a `BasicCoverageBlock` that the - // branching block would have given an `Expression` (or vice versa). - - let context = self - .context_stack - .iter_mut() - .rev() - .find(|context| match context.loop_header { - Some(loop_header) => { - self.basic_coverage_blocks.dominates(loop_header, successor) - } - None => true, - }) - .unwrap_or_else(|| bug!("should always fall back to the root non-loop context")); - debug!("adding to worklist for {:?}", context.loop_header); - - // FIXME: The code below had debug messages claiming to add items to a - // particular end of the worklist, but was confused about which end was - // which. The existing behaviour has been preserved for now, but it's - // unclear what the intended behaviour was. - - if self.basic_coverage_blocks.successors[successor].len() > 1 { - context.worklist.push_back(successor); - } else { - context.worklist.push_front(successor); - } - } - } - - pub(crate) fn is_complete(&self) -> bool { - self.visited.count() == self.visited.domain_size() - } - - pub(crate) fn unvisited(&self) -> Vec<BasicCoverageBlock> { - let mut unvisited_set: BitSet<BasicCoverageBlock> = - BitSet::new_filled(self.visited.domain_size()); - unvisited_set.subtract(&self.visited); - unvisited_set.iter().collect::<Vec<_>>() - } -} - /// Wrapper around a [`mir::BasicBlocks`] graph that restricts each node's /// successors to only the ones considered "relevant" when building a coverage /// graph. @@ -622,3 +489,126 @@ impl<'a, 'tcx> graph::Successors for CoverageRelevantSubgraph<'a, 'tcx> { self.coverage_successors(bb).into_iter() } } + +/// State of a node in the coverage graph during ready-first traversal. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +enum ReadyState { + /// This node has not yet been added to the fallback queue or ready queue. + Unqueued, + /// This node is currently in the fallback queue. + InFallbackQueue, + /// This node's predecessors have all been visited, so it is in the ready queue. + /// (It might also have a stale entry in the fallback queue.) + InReadyQueue, + /// This node has been visited. + /// (It might also have a stale entry in the fallback queue.) + Visited, +} + +/// Iterator that visits nodes in the coverage graph, in an order that always +/// prefers "ready" nodes whose predecessors have already been visited. +pub(crate) struct ReadyFirstTraversal<'a> { + graph: &'a CoverageGraph, + + /// For each node, the number of its predecessor nodes that haven't been visited yet. + n_unvisited_preds: IndexVec<BasicCoverageBlock, u32>, + /// Indicates whether a node has been visited, or which queue it is in. + state: IndexVec<BasicCoverageBlock, ReadyState>, + + /// Holds unvisited nodes whose predecessors have all been visited. + ready_queue: VecDeque<BasicCoverageBlock>, + /// Holds unvisited nodes with some unvisited predecessors. + /// Also contains stale entries for nodes that were upgraded to ready. + fallback_queue: VecDeque<BasicCoverageBlock>, +} + +impl<'a> ReadyFirstTraversal<'a> { + pub(crate) fn new(graph: &'a CoverageGraph) -> Self { + let num_nodes = graph.num_nodes(); + + let n_unvisited_preds = + IndexVec::from_fn_n(|node| graph.predecessors[node].len() as u32, num_nodes); + let mut state = IndexVec::from_elem_n(ReadyState::Unqueued, num_nodes); + + // We know from coverage graph construction that the start node is the + // only node with no predecessors. + debug_assert!( + n_unvisited_preds.iter_enumerated().all(|(node, &n)| (node == START_BCB) == (n == 0)) + ); + let ready_queue = VecDeque::from(vec![START_BCB]); + state[START_BCB] = ReadyState::InReadyQueue; + + Self { graph, state, n_unvisited_preds, ready_queue, fallback_queue: VecDeque::new() } + } + + /// Returns the next node from the ready queue, or else the next unvisited + /// node from the fallback queue. + fn next_inner(&mut self) -> Option<BasicCoverageBlock> { + // Always prefer to yield a ready node if possible. + if let Some(node) = self.ready_queue.pop_front() { + assert_eq!(self.state[node], ReadyState::InReadyQueue); + return Some(node); + } + + while let Some(node) = self.fallback_queue.pop_front() { + match self.state[node] { + // This entry in the fallback queue is not stale, so yield it. + ReadyState::InFallbackQueue => return Some(node), + // This node was added to the fallback queue, but later became + // ready and was visited via the ready queue. Ignore it here. + ReadyState::Visited => {} + // Unqueued nodes can't be in the fallback queue, by definition. + // We know that the ready queue is empty at this point. + ReadyState::Unqueued | ReadyState::InReadyQueue => unreachable!( + "unexpected state for {node:?} in the fallback queue: {:?}", + self.state[node] + ), + } + } + + None + } + + fn mark_visited_and_enqueue_successors(&mut self, node: BasicCoverageBlock) { + assert!(self.state[node] < ReadyState::Visited); + self.state[node] = ReadyState::Visited; + + // For each of this node's successors, decrease the successor's + // "unvisited predecessors" count, and enqueue it if appropriate. + for &succ in &self.graph.successors[node] { + let is_unqueued = match self.state[succ] { + ReadyState::Unqueued => true, + ReadyState::InFallbackQueue => false, + ReadyState::InReadyQueue => { + unreachable!("nodes in the ready queue have no unvisited predecessors") + } + // The successor was already visited via one of its other predecessors. + ReadyState::Visited => continue, + }; + + self.n_unvisited_preds[succ] -= 1; + if self.n_unvisited_preds[succ] == 0 { + // This node's predecessors have all been visited, so add it to + // the ready queue. If it's already in the fallback queue, that + // fallback entry will be ignored later. + self.state[succ] = ReadyState::InReadyQueue; + self.ready_queue.push_back(succ); + } else if is_unqueued { + // This node has unvisited predecessors, so add it to the + // fallback queue in case we run out of ready nodes later. + self.state[succ] = ReadyState::InFallbackQueue; + self.fallback_queue.push_back(succ); + } + } + } +} + +impl<'a> Iterator for ReadyFirstTraversal<'a> { + type Item = BasicCoverageBlock; + + fn next(&mut self) -> Option<Self::Item> { + let node = self.next_inner()?; + self.mark_visited_and_enqueue_successors(node); + Some(node) + } +} diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 0090f6f3040..edaec3c7965 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -1,8 +1,11 @@ use rustc_data_structures::captures::Captures; use rustc_index::bit_set::BitSet; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; -use rustc_middle::mir::coverage::{CovTerm, CoverageKind, MappingKind}; -use rustc_middle::mir::{Body, CoverageIdsInfo, Statement, StatementKind}; +use rustc_middle::mir::coverage::{ + CounterId, CovTerm, CoverageIdsInfo, CoverageKind, Expression, ExpressionId, + FunctionCoverageInfo, MappingKind, Op, +}; +use rustc_middle::mir::{Body, Statement, StatementKind}; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::util::Providers; @@ -87,10 +90,10 @@ fn coverage_ids_info<'tcx>( ) -> CoverageIdsInfo { let mir_body = tcx.instance_mir(instance_def); - let Some(fn_cov_info) = mir_body.function_coverage_info.as_ref() else { + let Some(fn_cov_info) = mir_body.function_coverage_info.as_deref() else { return CoverageIdsInfo { counters_seen: BitSet::new_empty(0), - expressions_seen: BitSet::new_empty(0), + zero_expressions: BitSet::new_empty(0), }; }; @@ -123,7 +126,10 @@ fn coverage_ids_info<'tcx>( } } - CoverageIdsInfo { counters_seen, expressions_seen } + let zero_expressions = + identify_zero_expressions(fn_cov_info, &counters_seen, &expressions_seen); + + CoverageIdsInfo { counters_seen, zero_expressions } } fn all_coverage_in_mir_body<'a, 'tcx>( @@ -141,3 +147,94 @@ fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { let scope_data = &body.source_scopes[statement.source_info.scope]; scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() } + +/// Identify expressions that will always have a value of zero, and note +/// their IDs in a `BitSet`. Mappings that refer to a zero expression +/// can instead become mappings to a constant zero value. +/// +/// This function mainly exists to preserve the simplifications that were +/// already being performed by the Rust-side expression renumbering, so that +/// the resulting coverage mappings don't get worse. +fn identify_zero_expressions( + fn_cov_info: &FunctionCoverageInfo, + counters_seen: &BitSet<CounterId>, + expressions_seen: &BitSet<ExpressionId>, +) -> BitSet<ExpressionId> { + // The set of expressions that either were optimized out entirely, or + // have zero as both of their operands, and will therefore always have + // a value of zero. Other expressions that refer to these as operands + // can have those operands replaced with `CovTerm::Zero`. + let mut zero_expressions = BitSet::new_empty(fn_cov_info.expressions.len()); + + // Simplify a copy of each expression based on lower-numbered expressions, + // and then update the set of always-zero expressions if necessary. + // (By construction, expressions can only refer to other expressions + // that have lower IDs, so one pass is sufficient.) + for (id, expression) in fn_cov_info.expressions.iter_enumerated() { + if !expressions_seen.contains(id) { + // If an expression was not seen, it must have been optimized away, + // so any operand that refers to it can be replaced with zero. + zero_expressions.insert(id); + continue; + } + + // We don't need to simplify the actual expression data in the + // expressions list; we can just simplify a temporary copy and then + // use that to update the set of always-zero expressions. + let Expression { mut lhs, op, mut rhs } = *expression; + + // If an expression has an operand that is also an expression, the + // operand's ID must be strictly lower. This is what lets us find + // all zero expressions in one pass. + let assert_operand_expression_is_lower = |operand_id: ExpressionId| { + assert!( + operand_id < id, + "Operand {operand_id:?} should be less than {id:?} in {expression:?}", + ) + }; + + // If an operand refers to a counter or expression that is always + // zero, then that operand can be replaced with `CovTerm::Zero`. + let maybe_set_operand_to_zero = |operand: &mut CovTerm| { + if let CovTerm::Expression(id) = *operand { + assert_operand_expression_is_lower(id); + } + + if is_zero_term(&counters_seen, &zero_expressions, *operand) { + *operand = CovTerm::Zero; + } + }; + maybe_set_operand_to_zero(&mut lhs); + maybe_set_operand_to_zero(&mut rhs); + + // Coverage counter values cannot be negative, so if an expression + // involves subtraction from zero, assume that its RHS must also be zero. + // (Do this after simplifications that could set the LHS to zero.) + if lhs == CovTerm::Zero && op == Op::Subtract { + rhs = CovTerm::Zero; + } + + // After the above simplifications, if both operands are zero, then + // we know that this expression is always zero too. + if lhs == CovTerm::Zero && rhs == CovTerm::Zero { + zero_expressions.insert(id); + } + } + + zero_expressions +} + +/// Returns `true` if the given term is known to have a value of zero, taking +/// into account knowledge of which counters are unused and which expressions +/// are always zero. +fn is_zero_term( + counters_seen: &BitSet<CounterId>, + zero_expressions: &BitSet<ExpressionId>, + term: CovTerm, +) -> bool { + match term { + CovTerm::Zero => true, + CovTerm::Counter(id) => !counters_seen.contains(id), + CovTerm::Expression(id) => zero_expressions.contains(id), + } +} |