linker: add a SPIR-T-based zombie reporting alternative, behind --no-early-report-zombies.

This commit is contained in:
Eduard-Mihai Burtescu 2023-04-21 03:15:19 +03:00 committed by Eduard-Mihai Burtescu
parent dcd2c74054
commit ee8adbf566
7 changed files with 493 additions and 17 deletions

View File

@ -330,6 +330,11 @@ impl CodegenArgs {
"no-compact-ids",
"disables compaction of SPIR-V IDs at the end of linking",
);
opts.optflag(
"",
"no-early-report-zombies",
"delays reporting zombies (to allow more legalization)",
);
opts.optflag("", "no-structurize", "disables CFG structurization");
opts.optflag(
@ -509,6 +514,7 @@ impl CodegenArgs {
// FIXME(eddyb) clean up this `no-` "negation prefix" situation.
dce: !matches.opt_present("no-dce"),
compact_ids: !matches.opt_present("no-compact-ids"),
early_report_zombies: !matches.opt_present("no-early-report-zombies"),
structurize: !matches.opt_present("no-structurize"),
spirt: !matches.opt_present("no-spirt"),
spirt_passes: matches

View File

@ -2,6 +2,7 @@
//! the original codegen of a crate, and consumed by the `linker`.
use crate::builder_spirv::BuilderSpirv;
use either::Either;
use itertools::Itertools;
use rspirv::dr::{Instruction, Module, Operand};
use rspirv::spirv::{Decoration, Op, Word};
@ -14,7 +15,7 @@ use std::borrow::Cow;
use std::marker::PhantomData;
use std::ops::Range;
use std::path::PathBuf;
use std::{fmt, iter, slice};
use std::{fmt, iter, slice, str};
/// Decorations not native to SPIR-V require some form of encoding into existing
/// SPIR-V constructs, for which we use `OpDecorateString` with decoration type
@ -127,11 +128,13 @@ impl<'a> CustomDecoration<'a> for ZombieDecoration<'a> {
//
// NOTE(eddyb) by keeping `line`+`col`, we mimick `OpLine` limitations
// (which could be lifted in the future using custom SPIR-T debuginfo).
// NOTE(eddyb) `NonSemantic.Shader.DebugInfo`'s `DebugLine` has both start & end,
// might be good to invest in SPIR-T being able to use NonSemantic debuginfo.
#[derive(Copy, Clone)]
pub struct SrcLocDecoration<'a> {
file_name: &'a str,
line: u32,
col: u32,
pub file_name: &'a str,
pub line: u32,
pub col: u32,
}
impl<'a> CustomDecoration<'a> for SrcLocDecoration<'a> {
@ -184,7 +187,7 @@ impl<'tcx> SrcLocDecoration<'tcx> {
/// 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,
module: Either<&'a Module, &'a spirt::Module>,
src_loc_decorations: Option<FxIndexMap<Word, LazilyDecoded<'a, SrcLocDecoration<'a>>>>,
@ -203,8 +206,35 @@ struct SpvDebugInfo<'a> {
}
impl<'a> SpvDebugInfo<'a> {
fn collect(module: &'a Module) -> Self {
fn collect(module: Either<&'a Module, &'a spirt::Module>) -> Self {
let mut this = Self::default();
let module = match module {
Either::Left(module) => module,
// HACK(eddyb) the SPIR-T codepath is simpler, and kind of silly,
// but we need the `SpvDebugFile`'s `regenerated_rustc_source_file`
// caching, so for now it reuses `SpvDebugInfo` overall.
Either::Right(module) => {
let cx = module.cx_ref();
match &module.debug_info {
spirt::ModuleDebugInfo::Spv(debug_info) => {
for sources in debug_info.source_languages.values() {
for (&file_name, src) in &sources.file_contents {
// FIXME(eddyb) what if the file is already present,
// should it be considered ambiguous overall?
this.files
.entry(&cx[file_name])
.or_default()
.op_source_parts = [&src[..]].into_iter().collect();
}
}
}
}
return this;
}
};
let mut insts = module.debug_string_source.iter().peekable();
while let Some(inst) = insts.next() {
match inst.class.opcode {
@ -255,7 +285,19 @@ impl<'a> SpanRegenerator<'a> {
pub fn new(source_map: &'a SourceMap, module: &'a Module) -> Self {
Self {
source_map,
module,
module: Either::Left(module),
src_loc_decorations: None,
zombie_decorations: None,
spv_debug_info: None,
}
}
pub fn new_spirt(source_map: &'a SourceMap, module: &'a spirt::Module) -> Self {
Self {
source_map,
module: Either::Right(module),
src_loc_decorations: None,
zombie_decorations: None,
@ -266,7 +308,9 @@ impl<'a> SpanRegenerator<'a> {
pub fn src_loc_for_id(&mut self, id: Word) -> Option<SrcLocDecoration<'a>> {
self.src_loc_decorations
.get_or_insert_with(|| SrcLocDecoration::decode_all(self.module).collect())
.get_or_insert_with(|| {
SrcLocDecoration::decode_all(self.module.left().unwrap()).collect()
})
.get(&id)
.map(|src_loc| src_loc.decode())
}
@ -275,7 +319,9 @@ impl<'a> SpanRegenerator<'a> {
// to handle it together with `SrcLocDecoration`, than separately.
pub(crate) fn zombie_for_id(&mut self, id: Word) -> Option<ZombieDecoration<'a>> {
self.zombie_decorations
.get_or_insert_with(|| ZombieDecoration::decode_all(self.module).collect())
.get_or_insert_with(|| {
ZombieDecoration::decode_all(self.module.left().unwrap()).collect()
})
.get(&id)
.map(|zombie| zombie.decode())
}

View File

@ -38,6 +38,7 @@ pub type Result<T> = std::result::Result<T, ErrorGuaranteed>;
pub struct Options {
pub compact_ids: bool,
pub dce: bool,
pub early_report_zombies: bool,
pub structurize: bool,
pub spirt: bool,
pub spirt_passes: Vec<String>,
@ -214,20 +215,27 @@ pub fn link(
simple_passes::check_fragment_insts(sess, &output)?;
}
// HACK(eddyb) this has to run before the `remove_zombies` pass, so that any
// zombies that are passed as call arguments, but eventually unused, won't
// be (incorrectly) considered used.
// HACK(eddyb) this has to run before the `report_and_remove_zombies` pass,
// so that any zombies that are passed as call arguments, but eventually unused,
// won't be (incorrectly) considered used.
{
let _timer = sess.timer("link_remove_unused_params");
output = param_weakening::remove_unused_params(output);
}
{
let _timer = sess.timer("link_remove_zombies");
zombies::remove_zombies(sess, opts, &mut output)?;
if opts.early_report_zombies {
let _timer = sess.timer("link_report_and_remove_zombies");
zombies::report_and_remove_zombies(sess, opts, &mut output)?;
}
{
// HACK(eddyb) this is not the best approach, but storage class inference
// can still fail in entirely legitimate ways (i.e. mismatches in zombies).
if !opts.early_report_zombies {
let _timer = sess.timer("link_dce-before-specialize_generic_storage_class");
dce::dce(&mut output);
}
let _timer = sess.timer("specialize_generic_storage_class");
// HACK(eddyb) `specializer` requires functions' blocks to be in RPO order
// (i.e. `block_ordering_pass`) - this could be relaxed by using RPO visit
@ -411,6 +419,11 @@ pub fn link(
);
}
let report_diagnostics_result = {
let _timer = sess.timer("spirt_passes::diagnostics::report_diagnostics");
spirt_passes::diagnostics::report_diagnostics(sess, opts, &module)
};
// NOTE(eddyb) this should be *before* `lift_to_spv` below,
// so if that fails, the dump could be used to debug it.
if let Some(dump_dir) = &opts.dump_spirt_passes {
@ -446,6 +459,11 @@ pub fn link(
.unwrap();
}
// NOTE(eddyb) this is late so that `--dump-spirt-passes` is processed,
// even/especially when error were reported, but lifting to SPIR-V is
// skipped (since it could very well fail due to reported errors).
report_diagnostics_result?;
let spv_words = {
let _timer = sess.timer("spirt::Module::lift_to_spv_module_emitter");
module.lift_to_spv_module_emitter().unwrap().words

View File

@ -0,0 +1,383 @@
use crate::decorations::{CustomDecoration, SpanRegenerator, SrcLocDecoration, ZombieDecoration};
use rustc_data_structures::fx::FxIndexSet;
use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed};
use rustc_session::Session;
use rustc_span::{Span, DUMMY_SP};
use smallvec::SmallVec;
use spirt::visit::{InnerVisit, Visitor};
use spirt::{
spv, Attr, AttrSet, AttrSetDef, Const, Context, DataInstDef, DataInstKind, ExportKey, Exportee,
Func, GlobalVar, Module, Type,
};
use std::marker::PhantomData;
use std::{mem, str};
pub(crate) fn report_diagnostics(
sess: &Session,
linker_options: &crate::linker::Options,
module: &Module,
) -> crate::linker::Result<()> {
let cx = &module.cx();
let mut reporter = DiagnosticReporter {
sess,
linker_options,
cx,
module,
seen_attrs: FxIndexSet::default(),
seen_types: FxIndexSet::default(),
seen_consts: FxIndexSet::default(),
seen_global_vars: FxIndexSet::default(),
seen_funcs: FxIndexSet::default(),
use_stack: SmallVec::new(),
span_regen: SpanRegenerator::new_spirt(sess.source_map(), module),
overall_result: Ok(()),
};
for (export_key, &exportee) in &module.exports {
assert_eq!(reporter.use_stack.len(), 0);
if let Exportee::Func(func) = exportee {
let func_decl = &module.funcs[func];
reporter.use_stack.push(UseOrigin::IntraFunc {
func_attrs: func_decl.attrs,
func_export_key: Some(export_key),
inst_attrs: AttrSet::default(),
origin: IntraFuncUseOrigin::Other,
});
export_key.inner_visit_with(&mut reporter);
if reporter.seen_funcs.insert(func) {
reporter.visit_func_decl(func_decl);
}
reporter.use_stack.pop();
}
export_key.inner_visit_with(&mut reporter);
exportee.inner_visit_with(&mut reporter);
}
reporter.overall_result
}
// HACK(eddyb) version of `decorations::LazilyDecoded` that works for SPIR-T.
struct LazilyDecoded<D> {
encoded: String,
_marker: PhantomData<D>,
}
impl<D> LazilyDecoded<D> {
fn decode<'a>(&'a self) -> D
where
D: CustomDecoration<'a>,
{
D::decode(&self.encoded)
}
}
fn decode_spv_lit_str_with<R>(imms: &[spv::Imm], f: impl FnOnce(&str) -> R) -> R {
let wk = &super::SpvSpecWithExtras::get().well_known;
// FIXME(eddyb) deduplicate with `spirt::spv::extract_literal_string`.
let words = imms.iter().enumerate().map(|(i, &imm)| match (i, imm) {
(0, spirt::spv::Imm::Short(k, w) | spirt::spv::Imm::LongStart(k, w))
| (1.., spirt::spv::Imm::LongCont(k, w)) => {
// FIXME(eddyb) use `assert_eq!` after updating to latest SPIR-T.
assert!(k == wk.LiteralString);
w
}
_ => unreachable!(),
});
let bytes: SmallVec<[u8; 64]> = words
.flat_map(u32::to_le_bytes)
.take_while(|&byte| byte != 0)
.collect();
f(str::from_utf8(&bytes).expect("invalid UTF-8 in string literal"))
}
fn try_decode_custom_decoration<'a, D: CustomDecoration<'a>>(
attrs_def: &AttrSetDef,
) -> Option<LazilyDecoded<D>> {
let wk = &super::SpvSpecWithExtras::get().well_known;
attrs_def.attrs.iter().find_map(|attr| {
let spv_inst = match attr {
Attr::SpvAnnotation(spv_inst) if spv_inst.opcode == wk.OpDecorateString => spv_inst,
_ => return None,
};
let str_imms = spv_inst
.imms
.strip_prefix(&[spv::Imm::Short(wk.Decoration, wk.UserTypeGOOGLE)])?;
decode_spv_lit_str_with(str_imms, |prefixed_encoded| {
let encoded = prefixed_encoded.strip_prefix(D::ENCODING_PREFIX)?;
Some(LazilyDecoded {
encoded: encoded.to_string(),
_marker: PhantomData,
})
})
})
}
// FIXME(eddyb) this looks a lot like `ReachableUseCollector`, maybe some
// automation should be built around "deep visitors" in general?
struct DiagnosticReporter<'a> {
sess: &'a Session,
linker_options: &'a crate::linker::Options,
cx: &'a Context,
module: &'a Module,
seen_attrs: FxIndexSet<AttrSet>,
seen_types: FxIndexSet<Type>,
seen_consts: FxIndexSet<Const>,
seen_global_vars: FxIndexSet<GlobalVar>,
seen_funcs: FxIndexSet<Func>,
use_stack: SmallVec<[UseOrigin<'a>; 8]>,
span_regen: SpanRegenerator<'a>,
overall_result: crate::linker::Result<()>,
}
enum UseOrigin<'a> {
Global {
kind: &'static &'static str,
attrs: AttrSet,
},
IntraFunc {
func_attrs: AttrSet,
func_export_key: Option<&'a ExportKey>,
inst_attrs: AttrSet,
origin: IntraFuncUseOrigin,
},
}
enum IntraFuncUseOrigin {
CallCallee,
Other,
}
impl UseOrigin<'_> {
fn to_rustc_span(&self, cx: &Context, span_regen: &mut SpanRegenerator<'_>) -> Option<Span> {
let mut from_attrs = |attrs: AttrSet| {
let attrs_def = &cx[attrs];
attrs_def
.attrs
.iter()
.find_map(|attr| match attr {
&Attr::SpvDebugLine {
file_path,
line,
col,
} => span_regen.src_loc_to_rustc(SrcLocDecoration {
file_name: &cx[file_path.0],
line,
col,
}),
_ => None,
})
.or_else(|| {
span_regen.src_loc_to_rustc(
try_decode_custom_decoration::<SrcLocDecoration<'_>>(attrs_def)?.decode(),
)
})
};
match *self {
Self::Global { attrs, .. } => from_attrs(attrs),
Self::IntraFunc {
func_attrs,
inst_attrs,
..
} => from_attrs(inst_attrs).or_else(|| from_attrs(func_attrs)),
}
}
fn note(
&self,
cx: &Context,
span_regen: &mut SpanRegenerator<'_>,
err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
) {
let wk = &super::SpvSpecWithExtras::get().well_known;
let name_from_attrs = |attrs: AttrSet, kind| {
cx[attrs]
.attrs
.iter()
.find_map(|attr| match attr {
Attr::SpvAnnotation(spv_inst) if spv_inst.opcode == wk.OpName => {
Some(decode_spv_lit_str_with(&spv_inst.imms, |name| {
format!("`{name}`")
}))
}
_ => None,
})
.unwrap_or_else(|| format!("unnamed {kind}"))
};
let note = match self {
&Self::Global { kind, attrs } => {
format!("used by {}", name_from_attrs(attrs, *kind))
}
Self::IntraFunc {
func_attrs,
func_export_key,
inst_attrs: _,
origin,
} => {
let func_desc = func_export_key
.map(|export_key| match export_key {
&ExportKey::LinkName(name) => format!("function export `{}`", &cx[name]),
ExportKey::SpvEntryPoint { imms, .. } => match imms[..] {
[em @ spv::Imm::Short(em_kind, _), ref name_imms @ ..] => {
// FIXME(eddyb) use `assert_eq!` after updating to latest SPIR-T.
assert!(em_kind == wk.ExecutionModel);
let em = spv::print::operand_from_imms([em]).concat_to_plain_text();
decode_spv_lit_str_with(name_imms, |name| {
format!(
"{} entry-point `{name}`",
em.strip_prefix("ExecutionModel.").unwrap()
)
})
}
_ => unreachable!(),
},
})
.unwrap_or_else(|| name_from_attrs(*func_attrs, "function"));
match origin {
IntraFuncUseOrigin::CallCallee => format!("called by {func_desc}"),
IntraFuncUseOrigin::Other => format!("used from within {func_desc}"),
}
}
};
let span = self.to_rustc_span(cx, span_regen).unwrap_or(DUMMY_SP);
err.span_note(span, note);
}
}
impl DiagnosticReporter<'_> {
fn report_from_attrs(&mut self, attrs: AttrSet) {
if attrs == AttrSet::default() {
return;
}
// Split off the last entry in `self.use_stack` if it's for the definition
// that `attrs` come from - this should almost always be the case, except
// for instructions inside a function body, or visitor bugs.
let (current_def, use_stack_for_def) = self
.use_stack
.split_last()
.filter(
|(
&UseOrigin::Global {
attrs: use_attrs, ..
}
| &UseOrigin::IntraFunc {
func_attrs: use_attrs,
..
},
_,
)| { use_attrs == attrs },
)
.map_or((None, &self.use_stack[..]), |(current, stack)| {
(Some(current), stack)
});
let attrs_def = &self.cx[attrs];
if !self.linker_options.early_report_zombies {
if let Some(zombie) = try_decode_custom_decoration::<ZombieDecoration<'_>>(attrs_def) {
let ZombieDecoration { reason } = zombie.decode();
let def_span = current_def
.and_then(|def| def.to_rustc_span(self.cx, &mut self.span_regen))
.unwrap_or(DUMMY_SP);
let mut err = self.sess.struct_span_err(def_span, reason);
for use_origin in use_stack_for_def.iter().rev() {
use_origin.note(self.cx, &mut self.span_regen, &mut err);
}
self.overall_result = Err(err.emit());
}
}
}
}
impl Visitor<'_> for DiagnosticReporter<'_> {
fn visit_attr_set_use(&mut self, attrs: AttrSet) {
// HACK(eddyb) this avoids reporting the same diagnostics more than once.
if self.seen_attrs.insert(attrs) {
self.report_from_attrs(attrs);
}
}
fn visit_type_use(&mut self, ty: Type) {
if self.seen_types.insert(ty) {
let ty_def = &self.cx[ty];
self.use_stack.push(UseOrigin::Global {
kind: &"type",
attrs: ty_def.attrs,
});
self.visit_type_def(ty_def);
self.use_stack.pop();
}
}
fn visit_const_use(&mut self, ct: Const) {
if self.seen_consts.insert(ct) {
let ct_def = &self.cx[ct];
self.use_stack.push(UseOrigin::Global {
kind: &"constant",
attrs: ct_def.attrs,
});
self.visit_const_def(ct_def);
self.use_stack.pop();
}
}
fn visit_global_var_use(&mut self, gv: GlobalVar) {
if self.seen_global_vars.insert(gv) {
let gv_decl = &self.module.global_vars[gv];
self.use_stack.push(UseOrigin::Global {
// FIXME(eddyb) may be a `&CONST`, or an interface variable,
// not necessarily an user variable, so this could be confusing.
kind: &"global variable",
attrs: gv_decl.attrs,
});
self.visit_global_var_decl(gv_decl);
self.use_stack.pop();
}
}
fn visit_func_use(&mut self, func: Func) {
if self.seen_funcs.insert(func) {
let func_decl = &self.module.funcs[func];
self.use_stack.push(UseOrigin::IntraFunc {
func_attrs: func_decl.attrs,
func_export_key: None,
inst_attrs: AttrSet::default(),
origin: IntraFuncUseOrigin::Other,
});
self.visit_func_decl(func_decl);
self.use_stack.pop();
}
}
fn visit_data_inst_def(&mut self, data_inst_def: &DataInstDef) {
match self.use_stack.last_mut() {
Some(UseOrigin::IntraFunc { inst_attrs, .. }) => *inst_attrs = data_inst_def.attrs,
_ => unreachable!(),
}
if let DataInstKind::FuncCall(func) = data_inst_def.kind {
let replace_origin = |this: &mut Self, new_origin| match this.use_stack.last_mut() {
Some(UseOrigin::IntraFunc { origin, .. }) => mem::replace(origin, new_origin),
_ => unreachable!(),
};
// HACK(eddyb) visit `func` early, to control its `use_stack`, with
// the later visit from `inner_visit_with` ignored as a duplicate.
let old_origin = replace_origin(self, IntraFuncUseOrigin::CallCallee);
self.visit_func_use(func);
replace_origin(self, old_origin);
}
data_inst_def.inner_visit_with(self);
}
}

View File

@ -1,5 +1,6 @@
//! SPIR-T pass infrastructure and supporting utilities.
pub(crate) mod diagnostics;
mod fuse_selects;
mod reduce;
@ -59,8 +60,17 @@ macro_rules! def_spv_spec_with_extra_well_known {
}
let spv_spec = spv::spec::Spec::get();
let wk = &spv_spec.well_known;
let decorations = match &spv_spec.operand_kinds[wk.Decoration] {
spv::spec::OperandKindDef::ValueEnum { variants } => variants,
_ => unreachable!(),
};
let lookup_fns = PerWellKnownGroup {
opcode: |name| spv_spec.instructions.lookup(name).unwrap(),
operand_kind: |name| spv_spec.operand_kinds.lookup(name).unwrap(),
decoration: |name| decorations.lookup(name).unwrap().into(),
};
SpvSpecWithExtras {
@ -87,6 +97,12 @@ def_spv_spec_with_extra_well_known! {
OpCompositeInsert,
OpCompositeExtract,
],
operand_kind: spv::spec::OperandKind = [
ExecutionModel,
],
decoration: u32 = [
UserTypeGOOGLE,
],
}
/// Run intra-function passes on all `Func` definitions in the `Module`.
@ -112,7 +128,8 @@ pub(super) fn run_func_passes<P>(
seen_global_vars: FxIndexSet::default(),
seen_funcs: FxIndexSet::default(),
};
for &exportee in module.exports.values() {
for (export_key, &exportee) in &module.exports {
export_key.inner_visit_with(&mut collector);
exportee.inner_visit_with(&mut collector);
}
collector.seen_funcs

View File

@ -308,7 +308,7 @@ impl<'a> ZombieReporter<'a> {
}
}
pub fn remove_zombies(
pub fn report_and_remove_zombies(
sess: &Session,
opts: &super::Options,
module: &mut Module,

View File

@ -135,6 +135,12 @@ Disables compaction of SPIR-V IDs at the end of linking. Causes absolutely ginor
emitted. Useful if you're println debugging IDs in the linker (although spirv-opt will compact them
anyway, be careful).
### `--no-early-report-zombies`
Delays reporting zombies (aka "deferred errors") even further, to allow more legalization.
Currently this also replaces the zombie reporting with a SPIR-T-based version
(which may become the default in the future).
### `--no-structurize`
Disables CFG structurization. Probably results in invalid modules.