Skip to content

Commit

Permalink
Replace printing codegen args with tracing.
Browse files Browse the repository at this point in the history 
This removes the following options and replaces them with `tracing`:

* `--specializer-debug` => RUSTGPU_LOG=rustc_codegen_spirv::specializer=debug
* `--print-zombie` => RUSTGPU_LOG=print_zombie=debug
* `--print-all-zombie` => RUSTGPU_LOG=print_all_zombie=debug
  • Loading branch information
@LegNeato
LegNeato committed Jan 3, 2025
1 parent cc9b40d commit 0f38dad
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 136 deletions.
14 changes: 0 additions & 14 deletions crates/rustc_codegen_spirv/src/codegen_cx/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 1 addition & 4 deletions crates/rustc_codegen_spirv/src/linker/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>,
pub print_all_zombie: bool,
pub print_zombie: bool,
}

pub enum LinkResult {
Expand Down Expand Up @@ -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 {
Expand Down
147 changes: 61 additions & 86 deletions crates/rustc_codegen_spirv/src/linker/specializer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result>(F);
Expand Down Expand Up @@ -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()
Expand All @@ -131,10 +130,7 @@ pub fn specialize(

let mut specializer = Specializer {
specialization,

debug,
debug_names,

generics: IndexMap::new(),
int_consts: FxHashMap::default(),
};
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -537,9 +527,6 @@ struct Generic {
struct Specializer<S: Specialization> {
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<Word, String>,

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2032,24 +2015,20 @@ 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`
// (if the `OpTypeFunction` is "generic", that is).
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`.
Expand All @@ -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 {
Expand Down Expand Up @@ -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`.
Expand Down
30 changes: 17 additions & 13 deletions crates/rustc_codegen_spirv/src/linker/zombies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -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:?}"
);
}
}
}
Expand Down
Loading

0 comments on commit 0f38dad

Please sign in to comment.