diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index f806a5e3a3..bd5f4935b3 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs @@ -469,23 +469,12 @@ impl CodegenArgs { "spirt-keep-unstructured-cfg-in-dumps", "include initial unstructured CFG when dumping SPIR-T", ); - opts.optflag( - "", - "specializer-debug", - "enable debug logging for the specializer", - ); opts.optopt( "", "specializer-dump-instances", "dump all instances inferred by the specializer, to FILE", "FILE", ); - opts.optflag("", "print-all-zombie", "prints all removed zombies"); - opts.optflag( - "", - "print-zombie", - "prints everything removed (even transitively) due to zombies", - ); } // NOTE(eddyb) these are debugging options that used to be env vars @@ -639,10 +628,7 @@ impl CodegenArgs { .opt_present("spirt-keep-debug-sources-in-dumps"), spirt_keep_unstructured_cfg_in_dumps: matches .opt_present("spirt-keep-unstructured-cfg-in-dumps"), - specializer_debug: matches.opt_present("specializer-debug"), specializer_dump_instances: matches_opt_path("specializer-dump-instances"), - print_all_zombie: matches.opt_present("print-all-zombie"), - print_zombie: matches.opt_present("print-zombie"), }; Ok(Self { diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 036c1d920b..ff22e37928 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -63,10 +63,7 @@ pub struct Options { pub spirt_strip_custom_debuginfo_from_dumps: bool, pub spirt_keep_debug_sources_in_dumps: bool, pub spirt_keep_unstructured_cfg_in_dumps: bool, - pub specializer_debug: bool, pub specializer_dump_instances: Option, - pub print_all_zombie: bool, - pub print_zombie: bool, } pub enum LinkResult { @@ -316,7 +313,7 @@ pub fn link( } let _timer = sess.timer("link_report_and_remove_zombies"); - zombies::report_and_remove_zombies(sess, opts, &mut output)?; + zombies::report_and_remove_zombies(sess, &mut output)?; } if opts.infer_storage_classes { diff --git a/crates/rustc_codegen_spirv/src/linker/specializer.rs b/crates/rustc_codegen_spirv/src/linker/specializer.rs index 84114325d5..01c4c9f9b1 100644 --- a/crates/rustc_codegen_spirv/src/linker/specializer.rs +++ b/crates/rustc_codegen_spirv/src/linker/specializer.rs @@ -60,6 +60,7 @@ use smallvec::SmallVec; use std::collections::{BTreeMap, VecDeque}; use std::ops::{Range, RangeTo}; use std::{fmt, io, iter, mem, slice}; +use tracing::{debug, error}; // FIXME(eddyb) move this elsewhere. struct FmtBy) -> fmt::Result>(F); @@ -110,12 +111,10 @@ pub fn specialize( module: Module, specialization: impl Specialization, ) -> Module { - // FIXME(eddyb) use `log`/`tracing` instead. - let debug = opts.specializer_debug; let dump_instances = &opts.specializer_dump_instances; let mut debug_names = FxHashMap::default(); - if debug || dump_instances.is_some() { + if dump_instances.is_some() { debug_names = module .debug_names .iter() @@ -131,10 +130,7 @@ pub fn specialize( let mut specializer = Specializer { specialization, - - debug, debug_names, - generics: IndexMap::new(), int_consts: FxHashMap::default(), }; @@ -188,27 +184,21 @@ pub fn specialize( // For non-"generic" functions, we can apply `replacements` right away, // though not before finishing inference for all functions first // (because `expander` needs to borrow `specializer` immutably). - if debug { - eprintln!("non-generic replacements:"); - } + debug!("non-generic replacements:"); for (func_idx, replacements) in non_generic_replacements { let mut func = mem::replace( &mut expander.builder.module_mut().functions[func_idx], Function::new(), ); - if debug { - let empty = replacements.with_instance.is_empty() - && replacements.with_concrete_or_param.is_empty(); - if !empty { - eprintln!(" in %{}:", func.def_id().unwrap()); - } + let empty = + replacements.with_instance.is_empty() && replacements.with_concrete_or_param.is_empty(); + if !empty { + debug!(" in %{}:", func.def_id().unwrap()); } for (loc, operand) in replacements.to_concrete(&[], |instance| expander.alloc_instance_id(instance)) { - if debug { - eprintln!(" {operand} -> {loc:?}"); - } + debug!(" {operand} -> {loc:?}"); func.index_set(loc, operand.into()); } expander.builder.module_mut().functions[func_idx] = func; @@ -537,9 +527,6 @@ struct Generic { struct Specializer { specialization: S, - // FIXME(eddyb) use `log`/`tracing` instead. - debug: bool, - // HACK(eddyb) if debugging is requested, this is used to quickly get `OpName`s. debug_names: FxHashMap, @@ -1512,24 +1499,24 @@ impl InferError { // FIXME(eddyb) better error reporting than this. match self { Self::Conflict(a, b) => { - eprintln!("inference conflict: {a:?} vs {b:?}"); + error!("inference conflict: {a:?} vs {b:?}"); } } - eprint!(" in "); + error!(" in "); // FIXME(eddyb) deduplicate this with other instruction printing logic. if let Some(result_id) = inst.result_id { - eprint!("%{result_id} = "); + error!("%{result_id} = "); } - eprint!("Op{:?}", inst.class.opcode); + error!("Op{:?}", inst.class.opcode); for operand in inst .result_type .map(Operand::IdRef) .iter() .chain(inst.operands.iter()) { - eprint!(" {operand}"); + error!(" {operand}"); } - eprintln!(); + error!(""); std::process::exit(1); } @@ -1937,38 +1924,36 @@ impl<'a, S: Specialization> InferCx<'a, S> { }; let debug_dump_if_enabled = |cx: &Self, prefix| { - if cx.specializer.debug { - let result_type = match inst.class.opcode { - // HACK(eddyb) workaround for `OpFunction`, see earlier HACK comment. - Op::Function => Some( - InferOperand::from_operand_and_generic_args( - &Operand::IdRef(inst.result_type.unwrap()), - inputs_generic_args.clone(), - cx, - ) - .0, - ), - _ => type_of_result.clone(), - }; - let inputs = InferOperandList { - operands: &inst.operands, - all_generic_args: inputs_generic_args.clone(), - transform: None, - }; + let result_type = match inst.class.opcode { + // HACK(eddyb) workaround for `OpFunction`, see earlier HACK comment. + Op::Function => Some( + InferOperand::from_operand_and_generic_args( + &Operand::IdRef(inst.result_type.unwrap()), + inputs_generic_args.clone(), + cx, + ) + .0, + ), + _ => type_of_result.clone(), + }; + let inputs = InferOperandList { + operands: &inst.operands, + all_generic_args: inputs_generic_args.clone(), + transform: None, + }; - if inst_loc != InstructionLocation::Module { - eprint!(" "); - } - eprint!("{prefix}"); - if let Some(result_id) = inst.result_id { - eprint!("%{result_id} = "); - } - eprint!("Op{:?}", inst.class.opcode); - for operand in result_type.into_iter().chain(inputs.iter(cx)) { - eprint!(" {}", operand.display_with_infer_cx(cx)); - } - eprintln!(); + if inst_loc != InstructionLocation::Module { + debug!(" "); + } + debug!("{prefix}"); + if let Some(result_id) = inst.result_id { + debug!("%{result_id} = "); + } + debug!("Op{:?}", inst.class.opcode); + for operand in result_type.into_iter().chain(inputs.iter(cx)) { + debug!(" {}", operand.display_with_infer_cx(cx)); } + debug!(""); }; // If we have some instruction signatures for `inst`, enforce them. @@ -1998,12 +1983,10 @@ impl<'a, S: Specialization> InferCx<'a, S> { ), }; - if self.specializer.debug { - if inst_loc != InstructionLocation::Module { - eprint!(" "); - } - eprintln!(" found {:?}", m.debug_with_infer_cx(self)); + if inst_loc != InstructionLocation::Module { + debug!(" "); } + debug!(" found {:?}", m.debug_with_infer_cx(self)); if let Err(e) = self.equate_match_findings(m) { e.report(inst); @@ -2032,14 +2015,12 @@ impl<'a, S: Specialization> InferCx<'a, S> { fn instantiate_function(&mut self, func: &'a Function) { let func_id = func.def_id().unwrap(); - if self.specializer.debug { - eprintln!(); - eprint!("specializer::instantiate_function(%{func_id}"); - if let Some(name) = self.specializer.debug_names.get(&func_id) { - eprint!(" {name}"); - } - eprintln!("):"); + debug!(""); + debug!("specializer::instantiate_function(%{func_id}"); + if let Some(name) = self.specializer.debug_names.get(&func_id) { + debug!(" {name}"); } + debug!("):"); // Instantiate the defining `OpFunction` first, so that the first // inference variables match the parameters from the `Generic` @@ -2047,9 +2028,7 @@ impl<'a, S: Specialization> InferCx<'a, S> { assert!(self.infer_var_values.is_empty()); self.instantiate_instruction(func.def.as_ref().unwrap(), InstructionLocation::Module); - if self.specializer.debug { - eprintln!("infer body {{"); - } + debug!("infer body {{"); // If the `OpTypeFunction` is indeed "generic", we have to extract the // return / parameter types for `OpReturnValue` and `OpFunctionParameter`. @@ -2074,14 +2053,12 @@ impl<'a, S: Specialization> InferCx<'a, S> { let (i, param) = params.next().unwrap(); assert_eq!(param.class.opcode, Op::FunctionParameter); - if self.specializer.debug { - eprintln!( - " %{} = Op{:?} {}", - param.result_id.unwrap(), - param.class.opcode, - param_ty.display_with_infer_cx(self) - ); - } + debug!( + " %{} = Op{:?} {}", + param.result_id.unwrap(), + param.class.opcode, + param_ty.display_with_infer_cx(self) + ); self.record_instantiated_operand( OperandLocation { @@ -2133,13 +2110,11 @@ impl<'a, S: Specialization> InferCx<'a, S> { } } - if self.specializer.debug { - eprint!("}}"); - if let Some(func_ty) = self.type_of_result.get(&func_id) { - eprint!(" -> %{}: {}", func_id, func_ty.display_with_infer_cx(self)); - } - eprintln!(); + debug!("}}"); + if let Some(func_ty) = self.type_of_result.get(&func_id) { + debug!(" -> %{}: {}", func_id, func_ty.display_with_infer_cx(self)); } + debug!(""); } /// Helper for `into_replacements`, that computes a single `ConcreteOrParam`. diff --git a/crates/rustc_codegen_spirv/src/linker/zombies.rs b/crates/rustc_codegen_spirv/src/linker/zombies.rs index 9b9423a4b9..207548c38f 100644 --- a/crates/rustc_codegen_spirv/src/linker/zombies.rs +++ b/crates/rustc_codegen_spirv/src/linker/zombies.rs @@ -10,6 +10,7 @@ use rustc_errors::Diag; use rustc_session::Session; use rustc_span::{DUMMY_SP, Span}; use smallvec::SmallVec; +use tracing::{Level, debug}; #[derive(Copy, Clone)] struct Zombie<'a> { @@ -321,11 +322,7 @@ impl<'a> ZombieReporter<'a> { } } -pub fn report_and_remove_zombies( - sess: &Session, - opts: &super::Options, - module: &mut Module, -) -> super::Result<()> { +pub fn report_and_remove_zombies(sess: &Session, module: &mut Module) -> super::Result<()> { let mut zombies = Zombies { // FIXME(eddyb) avoid repeating this across different passes/helpers. custom_ext_inst_set_import: module @@ -345,9 +342,7 @@ pub fn report_and_remove_zombies( while zombies.spread(module) {} let result = ZombieReporter::new(sess, module, &zombies).report_all(); - - // FIXME(eddyb) use `log`/`tracing` instead. - if opts.print_all_zombie { + if tracing::enabled!(target: "print_all_zombie", Level::DEBUG) { let mut span_regen = SpanRegenerator::new(sess.source_map(), module); for &zombie_id in zombies.id_to_zombie_kind.keys() { let mut zombie_leaf_id = zombie_id; @@ -362,15 +357,21 @@ pub fn report_and_remove_zombies( } let reason = span_regen.zombie_for_id(zombie_leaf_id).unwrap().reason; - eprint!("zombie'd %{zombie_id} because {reason}"); + debug!( + target: "print_all_zombie", + "zombie'd %{zombie_id} because {reason}" + ); if !infection_chain.is_empty() { - eprint!(" (infected via {:?})", infection_chain); + debug!( + target: "print_all_zombie", + " (infected via {:?})", infection_chain + ); } - eprintln!(); + debug!(target: "print_all_zombie", ""); } } - if opts.print_zombie { + if tracing::enabled!(target: "print_zombie", Level::DEBUG) { let mut span_regen = SpanRegenerator::new(sess.source_map(), module); let names = get_names(module); for f in &module.functions { @@ -386,7 +387,10 @@ pub fn report_and_remove_zombies( let name = get_name(&names, f.def_id().unwrap()); let reason = span_regen.zombie_for_id(zombie_leaf_id).unwrap().reason; - eprintln!("function removed {name:?} because {reason:?}"); + debug!( + target: "print_zombie", + "function removed {name:?} because {reason:?}" + ); } } } diff --git a/docs/src/codegen-args.md b/docs/src/codegen-args.md index 18794a7dfe..df3bc061bc 100644 --- a/docs/src/codegen-args.md +++ b/docs/src/codegen-args.md @@ -93,29 +93,10 @@ splitting option, hence it takes a directory instead of a file path. This is the binary before `spirv-opt` is executed, so it may be useful to output this to check if an issue is in Rust-GPU, or in `spirv-opt`. -### `--specializer-debug` - -Enables debug logging for the specializer. - -_**FIXME(@eddyb)** use `log`/`tracing` instead for this purpose_ - ### `--specializer-dump-instances FILE` Dumps to `FILE` all instances inferred by the specializer. -### `--print-zombie` - -Prints to rustc stdout which functions were removed due to being zombies, and why. - -_**FIXME(@eddyb)** use `log`/`tracing` instead for this purpose_ - -### `--print-all-zombie` - -Prints to rustc stdout *everything* that was removed due to being zombies, why, and if it was an -original zombie or if it was infected. (prints a lot!) - -_**FIXME(@eddyb)** use `log`/`tracing` instead for this purpose_ - ### `--no-spirv-val` Disables running `spirv-val` on the final output. Spooky scary option, can cause invalid modules!