From a73fc021f24d509c4b3d10a3d33ac1789a705bae Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 18 Apr 2023 13:34:41 +0300 Subject: [PATCH] decorations: split off `SrcLocDecoration` from `ZombieDecoration`. --- .../src/builder/builder_methods.rs | 11 +- .../rustc_codegen_spirv/src/codegen_cx/mod.rs | 36 +++--- crates/rustc_codegen_spirv/src/decorations.rs | 99 ++++++++------- crates/rustc_codegen_spirv/src/linker/mod.rs | 9 ++ .../rustc_codegen_spirv/src/linker/zombies.rs | 120 ++++++++---------- 5 files changed, 146 insertions(+), 129 deletions(-) diff --git a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs index 439f39a304..d81a171dc8 100644 --- a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs +++ b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs @@ -656,8 +656,15 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { fn set_span(&mut self, span: Span) { self.current_span = Some(span); - let (file, line, col) = self.builder.file_line_col_for_op_line(span); - self.emit().line(file.file_name_op_string_id, line, col); + + // We may not always have valid spans. + // FIXME(eddyb) reduce the sources of this as much as possible. + if span.is_dummy() { + self.emit().no_line(); + } else { + let (file, line, col) = self.builder.file_line_col_for_op_line(span); + self.emit().line(file.file_name_op_string_id, line, col); + } } // FIXME(eddyb) change `Self::Function` to be more like a function index. diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index 1152bb3b57..3cc435f9e4 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs @@ -5,7 +5,7 @@ mod type_; use crate::builder::{ExtInst, InstructionTable}; use crate::builder_spirv::{BuilderCursor, BuilderSpirv, SpirvConst, SpirvValue, SpirvValueKind}; -use crate::decorations::{CustomDecoration, SerializedSpan, ZombieDecoration}; +use crate::decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration}; use crate::spirv_type::{SpirvType, SpirvTypePrinter, TypeCache}; use crate::symbols::Symbols; use crate::target::SpirvTarget; @@ -50,10 +50,11 @@ pub struct CodegenCx<'tcx> { /// Cache generated vtables pub vtables: RefCell, Option>), SpirvValue>>, pub ext_inst: RefCell, - /// Invalid spir-v IDs that should be stripped from the final binary, + /// Invalid SPIR-V IDs that should be stripped from the final binary, /// each with its own reason and span that should be used for reporting /// (in the event that the value is actually needed) - zombie_decorations: RefCell>>, + zombie_decorations: + RefCell, Option>)>>, /// Cache of all the builtin symbols we need pub sym: Rc, pub instruction_table: InstructionTable, @@ -187,12 +188,14 @@ impl<'tcx> CodegenCx<'tcx> { pub fn zombie_even_in_user_code(&self, word: Word, span: Span, reason: &str) { self.zombie_decorations.borrow_mut().insert( word, - ZombieDecoration { - // FIXME(eddyb) this could take advantage of `Cow` and use - // either `&'static str` or `String`, on a case-by-case basis. - reason: reason.to_string().into(), - span: SerializedSpan::from_rustc(span, &self.builder), - }, + ( + ZombieDecoration { + // FIXME(eddyb) this could take advantage of `Cow` and use + // either `&'static str` or `String`, on a case-by-case basis. + reason: reason.to_string().into(), + }, + SrcLocDecoration::from_rustc_span(span, &self.builder), + ), ); } @@ -224,12 +227,15 @@ impl<'tcx> CodegenCx<'tcx> { pub fn finalize_module(self) -> Module { let mut result = self.builder.finalize(); - result.annotations.extend( - self.zombie_decorations - .into_inner() - .into_iter() - .map(|(id, zombie)| zombie.encode(id)), - ); + result + .annotations + .extend(self.zombie_decorations.into_inner().into_iter().flat_map( + |(id, (zombie, src_loc))| { + [zombie.encode(id)] + .into_iter() + .chain(src_loc.map(|src_loc| src_loc.encode(id))) + }, + )); result } diff --git a/crates/rustc_codegen_spirv/src/decorations.rs b/crates/rustc_codegen_spirv/src/decorations.rs index 94aad34abb..b1075425e7 100644 --- a/crates/rustc_codegen_spirv/src/decorations.rs +++ b/crates/rustc_codegen_spirv/src/decorations.rs @@ -64,7 +64,7 @@ pub trait CustomDecoration: for<'de> Deserialize<'de> + Serialize { Some(( id, LazilyDeserialized { - json: json.into(), + json, _marker: PhantomData, }, )) @@ -96,65 +96,44 @@ type DecodeAllIter<'a, D> = iter::FilterMap< /// Helper allowing full deserialization to be avoided where possible. pub struct LazilyDeserialized<'a, D> { - json: Cow<'a, str>, + json: &'a str, _marker: PhantomData, } -impl Clone for LazilyDeserialized<'_, D> { - fn clone(&self) -> Self { - let Self { ref json, _marker } = *self; - Self { - json: json.clone(), - _marker, - } - } -} - -impl LazilyDeserialized<'_, D> { - pub fn deserialize<'a>(&'a self) -> D - where - D: Deserialize<'a>, - { - serde_json::from_str(&self.json).unwrap() - } - - pub fn into_owned(self) -> LazilyDeserialized<'static, D> { - let Self { json, _marker } = self; - LazilyDeserialized { - json: json.into_owned().into(), - _marker, - } +impl<'a, D: Deserialize<'a>> LazilyDeserialized<'a, D> { + pub fn deserialize(&self) -> D { + serde_json::from_str(self.json).unwrap() } } #[derive(Deserialize, Serialize)] pub struct ZombieDecoration<'a> { pub reason: Cow<'a, str>, - - #[serde(flatten)] - pub span: Option>, } impl CustomDecoration for ZombieDecoration<'_> { const ENCODING_PREFIX: &'static str = "Z"; } -/// Representation of a `rustc` `Span` that can be turned into a `Span` again -/// in another compilation, by regenerating the `rustc` `SourceFile`. +/// Equivalent of `OpLine`, for the places where `rspirv` currently doesn't let +/// us actually emit a real `OpLine`, the way generating SPIR-T directly might. // -// FIXME(eddyb) encode/decode this using a `file:line:col` string format. +// NOTE(eddyb) by keeping `line`+`col`, we mimick `OpLine` limitations +// (which could be lifted in the future using custom SPIR-T debuginfo). #[derive(Deserialize, Serialize)] -pub struct SerializedSpan<'a> { +pub struct SrcLocDecoration<'a> { file_name: Cow<'a, str>, - // NOTE(eddyb) by keeping `line`+`col`, we mimick `OpLine` limitations - // (which could be lifted in the future using custom SPIR-T debuginfo). line: u32, col: u32, } -impl<'tcx> SerializedSpan<'tcx> { - pub fn from_rustc(span: Span, builder: &BuilderSpirv<'tcx>) -> Option { - // Decorations may not always have valid spans. +impl CustomDecoration for SrcLocDecoration<'_> { + const ENCODING_PREFIX: &'static str = "L"; +} + +impl<'tcx> SrcLocDecoration<'tcx> { + pub fn from_rustc_span(span: Span, builder: &BuilderSpirv<'tcx>) -> Option { + // We may not always have valid spans. // FIXME(eddyb) reduce the sources of this as much as possible. if span.is_dummy() { return None; @@ -170,12 +149,18 @@ impl<'tcx> SerializedSpan<'tcx> { } } -/// Helper type to delay most of the work necessary to turn a `SerializedSpan` +/// Helper type to delay most of the work necessary to turn a `SrcLocDecoration` /// back into an usable `Span`, until it's actually needed (i.e. for an error). pub struct SpanRegenerator<'a> { source_map: &'a SourceMap, module: &'a Module, + src_loc_decorations: Option>>>, + + // HACK(eddyb) this has no really good reason to belong here, but it's easier + // to handle it together with `SrcLocDecoration`, than separately. + zombie_decorations: Option>>>, + // HACK(eddyb) this is mostly replicating SPIR-T's module-level debuginfo. spv_debug_files: Option>>, } @@ -194,10 +179,30 @@ impl<'a> SpanRegenerator<'a> { Self { source_map, module, + + src_loc_decorations: None, + zombie_decorations: None, + spv_debug_files: None, } } + pub fn src_loc_for_id(&mut self, id: Word) -> Option> { + self.src_loc_decorations + .get_or_insert_with(|| SrcLocDecoration::decode_all(self.module).collect()) + .get(&id) + .map(|src_loc| src_loc.deserialize()) + } + + // HACK(eddyb) this has no really good reason to belong here, but it's easier + // to handle it together with `SrcLocDecoration`, than separately. + pub(crate) fn zombie_for_id(&mut self, id: Word) -> Option> { + self.zombie_decorations + .get_or_insert_with(|| ZombieDecoration::decode_all(self.module).collect()) + .get(&id) + .map(|zombie| zombie.deserialize()) + } + fn regenerate_rustc_source_file(&mut self, file_name: &str) -> Option<&SourceFile> { let spv_debug_files = self.spv_debug_files.get_or_insert_with(|| { let mut op_string_by_id = FxHashMap::default(); @@ -287,10 +292,16 @@ impl<'a> SpanRegenerator<'a> { file.as_deref() } - pub fn serialized_span_to_rustc(&mut self, span: &SerializedSpan<'_>) -> Option { - let file = self.regenerate_rustc_source_file(&span.file_name[..])?; + pub fn src_loc_to_rustc(&mut self, src_loc: &SrcLocDecoration<'_>) -> Option { + let SrcLocDecoration { + ref file_name, + line, + col, + } = *src_loc; - let line_bpos_range = file.line_bounds(span.line.checked_sub(1)? as usize); + let file = self.regenerate_rustc_source_file(file_name)?; + + let line_bpos_range = file.line_bounds(line.checked_sub(1)? as usize); // Find the special cases (`MultiByteChar`s/`NonNarrowChar`s) in the line. let multibyte_chars = { @@ -320,7 +331,7 @@ impl<'a> SpanRegenerator<'a> { // (this may look inefficient, but lines tend to be short, and `rustc` // itself is even worse than this, when it comes to `BytePos` lookups). let (mut cur_bpos, mut cur_col_display) = (line_bpos_range.start, 0); - while cur_bpos < line_bpos_range.end && cur_col_display < span.col { + while cur_bpos < line_bpos_range.end && cur_col_display < col { let next_special_bpos = special_chars.peek().map(|special| { special .as_ref() @@ -332,7 +343,7 @@ impl<'a> SpanRegenerator<'a> { let following_trivial_chars = next_special_bpos.unwrap_or(line_bpos_range.end).0 - cur_bpos.0; if following_trivial_chars > 0 { - let wanted_trivial_chars = following_trivial_chars.min(span.col - cur_col_display); + let wanted_trivial_chars = following_trivial_chars.min(col - cur_col_display); cur_bpos.0 += wanted_trivial_chars; cur_col_display += wanted_trivial_chars; continue; diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 1536c307b5..9611d26fbe 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -20,6 +20,7 @@ mod zombies; use std::borrow::Cow; use crate::codegen_cx::SpirvMetadata; +use crate::decorations::{CustomDecoration, SrcLocDecoration, ZombieDecoration}; use either::Either; use rspirv::binary::{Assemble, Consumer}; use rspirv::dr::{Block, Instruction, Loader, Module, ModuleHeader, Operand}; @@ -572,6 +573,14 @@ pub fn link( // compact the ids https://github.com/KhronosGroup/SPIRV-Tools/blob/e02f178a716b0c3c803ce31b9df4088596537872/source/opt/compact_ids_pass.cpp#L43 output.header.as_mut().unwrap().bound = simple_passes::compact_ids(output); }; + + // FIXME(eddyb) convert these into actual `OpLine`s with a SPIR-T pass, + // but that'd require keeping the modules in SPIR-T form (once lowered), + // and never loading them back into `rspirv` once lifted back to SPIR-V. + SrcLocDecoration::remove_all(output); + + // FIXME(eddyb) might make more sense to rewrite these away on SPIR-T. + ZombieDecoration::remove_all(output); } Ok(output) diff --git a/crates/rustc_codegen_spirv/src/linker/zombies.rs b/crates/rustc_codegen_spirv/src/linker/zombies.rs index 0707796589..eb0dda7c47 100644 --- a/crates/rustc_codegen_spirv/src/linker/zombies.rs +++ b/crates/rustc_codegen_spirv/src/linker/zombies.rs @@ -1,7 +1,7 @@ //! See documentation on `CodegenCx::zombie` for a description of the zombie system. use super::{get_name, get_names}; -use crate::decorations::{CustomDecoration, LazilyDeserialized, SpanRegenerator, ZombieDecoration}; +use crate::decorations::{CustomDecoration, SpanRegenerator, ZombieDecoration}; use rspirv::dr::{Instruction, Module}; use rspirv::spirv::{Op, Word}; use rustc_data_structures::fx::FxHashMap; @@ -11,64 +11,61 @@ use std::iter::once; // FIXME(eddyb) change this to chain through IDs instead of wasting allocations. #[derive(Clone)] -struct ZombieInfo<'a> { - serialized: &'a LazilyDeserialized<'static, ZombieDecoration<'a>>, +struct Zombie { + leaf_id: Word, stack: Vec, } -impl<'a> ZombieInfo<'a> { +impl Zombie { fn push_stack(&self, word: Word) -> Self { Self { - serialized: self.serialized, + leaf_id: self.leaf_id, stack: self.stack.iter().cloned().chain(once(word)).collect(), } } } -fn contains_zombie<'h, 'a>( +fn contains_zombie<'h>( inst: &Instruction, - zombie: &'h FxHashMap>, -) -> Option<&'h ZombieInfo<'a>> { + zombies: &'h FxHashMap, +) -> Option<&'h Zombie> { if let Some(result_type) = inst.result_type { - if let Some(reason) = zombie.get(&result_type) { + if let Some(reason) = zombies.get(&result_type) { return Some(reason); } } inst.operands .iter() - .find_map(|op| op.id_ref_any().and_then(|w| zombie.get(&w))) + .find_map(|op| op.id_ref_any().and_then(|w| zombies.get(&w))) } -fn is_zombie<'h, 'a>( - inst: &Instruction, - zombie: &'h FxHashMap>, -) -> Option<&'h ZombieInfo<'a>> { +fn is_zombie<'h>(inst: &Instruction, zombies: &'h FxHashMap) -> Option<&'h Zombie> { if let Some(result_id) = inst.result_id { - zombie.get(&result_id) + zombies.get(&result_id) } else { - contains_zombie(inst, zombie) + contains_zombie(inst, zombies) } } -fn is_or_contains_zombie<'h, 'a>( +fn is_or_contains_zombie<'h>( inst: &Instruction, - zombie: &'h FxHashMap>, -) -> Option<&'h ZombieInfo<'a>> { - let result_zombie = inst.result_id.and_then(|result_id| zombie.get(&result_id)); - result_zombie.or_else(|| contains_zombie(inst, zombie)) + zombies: &'h FxHashMap, +) -> Option<&'h Zombie> { + let result_zombie = inst.result_id.and_then(|result_id| zombies.get(&result_id)); + result_zombie.or_else(|| contains_zombie(inst, zombies)) } -fn spread_zombie(module: &Module, zombie: &mut FxHashMap>) -> bool { +fn spread_zombie(module: &Module, zombies: &mut FxHashMap) -> bool { let mut any = false; // globals are easy for inst in module.global_inst_iter() { if let Some(result_id) = inst.result_id { - if let Some(reason) = contains_zombie(inst, zombie) { - if zombie.contains_key(&result_id) { + if let Some(reason) = contains_zombie(inst, zombies) { + if zombies.contains_key(&result_id) { continue; } let reason = reason.clone(); - zombie.insert(result_id, reason); + zombies.insert(result_id, reason); any = true; } } @@ -81,24 +78,24 @@ fn spread_zombie(module: &Module, zombie: &mut FxHashMap>) for func in &module.functions { let func_id = func.def_id().unwrap(); // Can't use zombie.entry() here, due to using the map in contains_zombie - if zombie.contains_key(&func_id) { + if zombies.contains_key(&func_id) { // Func is already zombie, no need to scan it again. continue; } for inst in func.all_inst_iter() { if inst.class.opcode == Op::Variable { let result_id = inst.result_id.unwrap(); - if let Some(reason) = contains_zombie(inst, zombie) { - if zombie.contains_key(&result_id) { + if let Some(reason) = contains_zombie(inst, zombies) { + if zombies.contains_key(&result_id) { continue; } let reason = reason.clone(); - zombie.insert(result_id, reason); + zombies.insert(result_id, reason); any = true; } - } else if let Some(reason) = is_or_contains_zombie(inst, zombie) { + } else if let Some(reason) = is_or_contains_zombie(inst, zombies) { let pushed_reason = reason.push_stack(func_id); - zombie.insert(func_id, pushed_reason); + zombies.insert(func_id, pushed_reason); any = true; break; } @@ -113,20 +110,21 @@ fn spread_zombie(module: &Module, zombie: &mut FxHashMap>) fn report_error_zombies( sess: &Session, module: &Module, - zombie: &FxHashMap>, + zombies: &FxHashMap, ) -> super::Result<()> { let mut span_regen = SpanRegenerator::new(sess.source_map(), module); let mut result = Ok(()); let mut names = None; for root in super::dce::collect_roots(module) { - if let Some(zombie_info) = zombie.get(&root) { - let ZombieDecoration { reason, span } = zombie_info.serialized.deserialize(); - let span = span - .and_then(|span| span_regen.serialized_span_to_rustc(&span)) + if let Some(zombie) = zombies.get(&root) { + let reason = span_regen.zombie_for_id(zombie.leaf_id).unwrap().reason; + let span = span_regen + .src_loc_for_id(zombie.leaf_id) + .and_then(|src_loc| span_regen.src_loc_to_rustc(&src_loc)) .unwrap_or(DUMMY_SP); let names = names.get_or_insert_with(|| get_names(module)); - let stack = zombie_info + let stack = zombie .stack .iter() .map(|&s| get_name(names, s).into_owned()); @@ -145,20 +143,12 @@ pub fn remove_zombies( opts: &super::Options, module: &mut Module, ) -> super::Result<()> { - // FIXME(eddyb) combine these two steps to take the original strings, - // instead of effectively cloning them (via `.into_owned()`). - let zombies_owned = ZombieDecoration::decode_all(module) - .map(|(id, zombie)| (id, zombie.into_owned())) - .collect::>(); - ZombieDecoration::remove_all(module); - - let mut zombies = zombies_owned - .iter() - .map(|(id, serialized)| { + let mut zombies = ZombieDecoration::decode_all(module) + .map(|(id, _)| { ( - *id, - ZombieInfo { - serialized, + id, + Zombie { + leaf_id: id, stack: vec![], }, ) @@ -171,31 +161,25 @@ pub fn remove_zombies( // FIXME(eddyb) use `log`/`tracing` instead. if opts.print_all_zombie { - for (&zombie_id, zombie_info) in &zombies { - let orig = if zombies_owned.iter().any(|&(z, _)| z == zombie_id) { - "original" - } else { - "infected" - }; - println!( - "zombie'd {} because {} ({})", - zombie_id, - zombie_info.serialized.deserialize().reason, - orig - ); + let mut span_regen = SpanRegenerator::new(sess.source_map(), module); + for (&zombie_id, zombie) in &zombies { + let reason = span_regen.zombie_for_id(zombie.leaf_id).unwrap().reason; + eprint!("zombie'd %{zombie_id} because {reason}"); + if zombie_id != zombie.leaf_id { + eprint!(" (infected by %{})", zombie.leaf_id); + } + eprintln!(); } } if opts.print_zombie { + let mut span_regen = SpanRegenerator::new(sess.source_map(), module); let names = get_names(module); for f in &module.functions { - if let Some(zombie_info) = is_zombie(f.def.as_ref().unwrap(), &zombies) { + if let Some(zombie) = is_zombie(f.def.as_ref().unwrap(), &zombies) { let name = get_name(&names, f.def_id().unwrap()); - println!( - "Function removed {:?} because {:?}", - name, - zombie_info.serialized.deserialize().reason - ); + let reason = span_regen.zombie_for_id(zombie.leaf_id).unwrap().reason; + eprintln!("function removed {name:?} because {reason:?}"); } } }