Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic structurization improvements #20

Merged
merged 4 commits into from
Oct 18, 2024
Merged

Panic structurization improvements #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae84c77
update spirt to 0.4.0
eddyb Jan 31, 2024
195051d
tests: add test for long chain of conditional panics.
eddyb Aug 8, 2024
27b56f2
Further process paths on windows 2
LegNeato Sep 18, 2024
b2df554
Use `ExitInvocation` for panics (avoiding deeply nested structurizati…
eddyb Jul 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/rustc_codegen_spirv/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ rustc_codegen_spirv-types.workspace = true
rustc-demangle = "0.1.21"
sanitize-filename = "0.4"
smallvec = { version = "1.6.1", features = ["union"] }
spirt = "0.3.0"
spirt = "0.4.0"
spirv-tools.workspace = true
lazy_static = "1.4.0"
itertools = "0.10.5"
Expand Down
7 changes: 7 additions & 0 deletions crates/rustc_codegen_spirv/src/codegen_cx/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,11 @@ impl CodegenArgs {
"spirt-keep-debug-sources-in-dumps",
"keep file contents debuginfo when dumping SPIR-T",
);
opts.optflag(
"",
"spirt-keep-unstructured-cfg-in-dumps",
"include initial unstructured CFG when dumping SPIR-T",
);
opts.optflag(
"",
"specializer-debug",
Expand Down Expand Up @@ -629,6 +634,8 @@ impl CodegenArgs {
.opt_present("spirt-strip-custom-debuginfo-from-dumps"),
spirt_keep_debug_sources_in_dumps: matches
.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"),
Expand Down
10 changes: 9 additions & 1 deletion crates/rustc_codegen_spirv/src/linker/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub struct Options {
pub dump_spirt_passes: Option<PathBuf>,
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,
Expand Down Expand Up @@ -434,9 +435,16 @@ pub fn link(
}
}
};
after_pass("lower_from_spv", &module);
// HACK(eddyb) don't dump the unstructured state if not requested, as
// after SPIR-T 0.4.0 it's extremely verbose (due to def-use hermeticity).
if opts.spirt_keep_unstructured_cfg_in_dumps || !opts.structurize {
after_pass("lower_from_spv", &module);
}

// NOTE(eddyb) this *must* run on unstructured CFGs, to do its job.
// FIXME(eddyb) no longer relying on structurization, try porting this
// to replace custom aborts in `Block`s and inject `ExitInvocation`s
// after them (truncating the `Block` and/or parent region if necessary).
{
let _timer = sess.timer("spirt_passes::controlflow::convert_custom_aborts_to_unstructured_returns_in_entry_points");
spirt_passes::controlflow::convert_custom_aborts_to_unstructured_returns_in_entry_points(opts, &mut module);
Expand Down
61 changes: 35 additions & 26 deletions crates/rustc_codegen_spirv/src/linker/spirt_passes/controlflow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ use crate::custom_insts::{self, CustomInst, CustomOp};
use smallvec::SmallVec;
use spirt::func_at::FuncAt;
use spirt::{
cfg, spv, Attr, AttrSet, ConstCtor, ConstDef, ControlNodeKind, DataInstFormDef, DataInstKind,
DeclDef, EntityDefs, ExportKey, Exportee, Module, Type, TypeCtor, TypeCtorArg, TypeDef, Value,
cfg, spv, Attr, AttrSet, ConstDef, ConstKind, ControlNodeKind, DataInstFormDef, DataInstKind,
DeclDef, EntityDefs, ExportKey, Exportee, Module, Type, TypeDef, TypeKind, TypeOrConst, Value,
};
use std::fmt::Write as _;

/// Replace our custom extended instruction `Abort`s with standard `OpReturn`s,
/// but only in entry-points (and only before CFG structurization).
//
// FIXME(eddyb) no longer relying on structurization, try porting this
// to replace custom aborts in `Block`s and inject `ExitInvocation`s
// after them (truncating the `Block` and/or parent region if necessary).
pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
linker_options: &crate::linker::Options,
module: &mut Module,
Expand Down Expand Up @@ -66,8 +70,8 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
};

let func_decl = &mut module.funcs[func];
assert!(match &cx[func_decl.ret_type].ctor {
TypeCtor::SpvInst(spv_inst) => spv_inst.opcode == wk.OpTypeVoid,
assert!(match &cx[func_decl.ret_type].kind {
TypeKind::SpvInst { spv_inst, .. } => spv_inst.opcode == wk.OpTypeVoid,
_ => false,
});

Expand Down Expand Up @@ -112,7 +116,7 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
if let DataInstKind::SpvInst(spv_inst) = &data_inst_form_def.kind {
if spv_inst.opcode == wk.OpLoad {
if let Value::Const(ct) = data_inst_def.inputs[0] {
if let ConstCtor::PtrToGlobalVar(gv) = cx[ct].ctor {
if let ConstKind::PtrToGlobalVar(gv) = cx[ct].kind {
if interface_global_vars.contains(&gv) {
return Some((
gv,
Expand All @@ -129,8 +133,8 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
if inputs {
let mut first_input = true;
for (gv, ty, value) in loaded_inputs {
let scalar_type = |ty: Type| match &cx[ty].ctor {
TypeCtor::SpvInst(spv_inst) => match spv_inst.imms[..] {
let scalar_type = |ty: Type| match &cx[ty].kind {
TypeKind::SpvInst { spv_inst, .. } => match spv_inst.imms[..] {
[spv::Imm::Short(_, 32), spv::Imm::Short(_, signedness)]
if spv_inst.opcode == wk.OpTypeInt =>
{
Expand All @@ -145,14 +149,16 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
};
let vector_or_scalar_type = |ty: Type| {
let ty_def = &cx[ty];
match (&ty_def.ctor, &ty_def.ctor_args[..]) {
(TypeCtor::SpvInst(spv_inst), &[TypeCtorArg::Type(elem)])
if spv_inst.opcode == wk.OpTypeVector =>
{
match spv_inst.imms[..] {
[spv::Imm::Short(_, vlen @ 2..=4)] => {
Some((scalar_type(elem)?, Some(vlen)))
}
match &ty_def.kind {
TypeKind::SpvInst {
spv_inst,
type_and_const_inputs,
} if spv_inst.opcode == wk.OpTypeVector => {
match (&type_and_const_inputs[..], &spv_inst.imms[..]) {
(
&[TypeOrConst::Type(elem)],
&[spv::Imm::Short(_, vlen @ 2..=4)],
) => Some((scalar_type(elem)?, Some(vlen))),
_ => None,
}
}
Expand Down Expand Up @@ -250,7 +256,9 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
)) = custom_terminator_inst
{
let abort_inst = func_at_abort_inst.position;
terminator.kind = cfg::ControlInstKind::Return;
terminator.kind = cfg::ControlInstKind::ExitInvocation(
cfg::ExitInvocationKind::SpvInst(wk.OpReturn.into()),
);

match abort_strategy {
Some(Strategy::Unreachable) => {
Expand All @@ -260,16 +268,19 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
inputs: _,
backtrace,
}) => {
let const_ctor = |v: Value| match v {
Value::Const(ct) => &cx[ct].ctor,
let const_kind = |v: Value| match v {
Value::Const(ct) => &cx[ct].kind,
_ => unreachable!(),
};
let const_str = |v: Value| match const_ctor(v) {
&ConstCtor::SpvStringLiteralForExtInst(s) => s,
let const_str = |v: Value| match const_kind(v) {
&ConstKind::SpvStringLiteralForExtInst(s) => s,
_ => unreachable!(),
};
let const_u32 = |v: Value| match const_ctor(v) {
ConstCtor::SpvInst(spv_inst) => {
let const_u32 = |v: Value| match const_kind(v) {
ConstKind::SpvInst {
spv_inst_and_const_inputs,
} => {
let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs;
assert!(spv_inst.opcode == wk.OpConstant);
match spv_inst.imms[..] {
[spv::Imm::Short(_, x)] => x,
Expand All @@ -283,11 +294,9 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points(
attrs: Default::default(),
ty: cx.intern(TypeDef {
attrs: Default::default(),
ctor: TypeCtor::SpvStringLiteralForExtInst,
ctor_args: Default::default(),
kind: TypeKind::SpvStringLiteralForExtInst,
}),
ctor: ConstCtor::SpvStringLiteralForExtInst(s),
ctor_args: Default::default(),
kind: ConstKind::SpvStringLiteralForExtInst(s),
})
};

Expand Down
18 changes: 11 additions & 7 deletions crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use smallvec::SmallVec;
use spirt::transform::{InnerInPlaceTransform, Transformer};
use spirt::visit::InnerVisit;
use spirt::{
spv, Attr, AttrSetDef, ConstCtor, Context, ControlNode, ControlNodeKind, DataInstKind,
spv, Attr, AttrSetDef, ConstKind, Context, ControlNode, ControlNodeKind, DataInstKind,
InternedStr, Module, OrdAssertEq, Value,
};

Expand Down Expand Up @@ -95,16 +95,20 @@ impl Transformer for CustomDebuginfoToSpv<'_> {
col_start: col,
col_end: _,
} => {
let const_ctor = |v: Value| match v {
Value::Const(ct) => &self.cx[ct].ctor,
let const_kind = |v: Value| match v {
Value::Const(ct) => &self.cx[ct].kind,
_ => unreachable!(),
};
let const_str = |v: Value| match const_ctor(v) {
&ConstCtor::SpvStringLiteralForExtInst(s) => s,
let const_str = |v: Value| match const_kind(v) {
&ConstKind::SpvStringLiteralForExtInst(s) => s,
_ => unreachable!(),
};
let const_u32 = |v: Value| match const_ctor(v) {
ConstCtor::SpvInst(spv_inst) => {
let const_u32 = |v: Value| match const_kind(v) {
ConstKind::SpvInst {
spv_inst_and_const_inputs,
} => {
let (spv_inst, _const_inputs) =
&**spv_inst_and_const_inputs;
assert!(spv_inst.opcode == self.wk.OpConstant);
match spv_inst.imms[..] {
[spv::Imm::Short(_, x)] => x,
Expand Down
29 changes: 16 additions & 13 deletions crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use smallvec::SmallVec;
use spirt::func_at::FuncAt;
use spirt::visit::{InnerVisit, Visitor};
use spirt::{
spv, Attr, AttrSet, AttrSetDef, Const, ConstCtor, Context, ControlNode, ControlNodeKind,
spv, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, ControlNode, ControlNodeKind,
DataInstDef, DataInstForm, DataInstKind, Diag, DiagLevel, ExportKey, Exportee, Func, FuncDecl,
GlobalVar, InternedStr, Module, Type, Value,
};
Expand Down Expand Up @@ -275,16 +275,19 @@ impl UseOrigin<'_> {
} => (file, line_start, line_end, col_start, col_end),
_ => unreachable!(),
};
let const_ctor = |v: Value| match v {
Value::Const(ct) => &cx[ct].ctor,
let const_kind = |v: Value| match v {
Value::Const(ct) => &cx[ct].kind,
_ => unreachable!(),
};
let const_str = |v: Value| match const_ctor(v) {
&ConstCtor::SpvStringLiteralForExtInst(s) => s,
let const_str = |v: Value| match const_kind(v) {
&ConstKind::SpvStringLiteralForExtInst(s) => s,
_ => unreachable!(),
};
let const_u32 = |v: Value| match const_ctor(v) {
ConstCtor::SpvInst(spv_inst) => {
let const_u32 = |v: Value| match const_kind(v) {
ConstKind::SpvInst {
spv_inst_and_const_inputs,
} => {
let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs;
assert!(spv_inst.opcode == wk.OpConstant);
match spv_inst.imms[..] {
[spv::Imm::Short(_, x)] => x,
Expand Down Expand Up @@ -505,9 +508,9 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> {
fn visit_const_use(&mut self, ct: Const) {
if self.seen_consts.insert(ct) {
let ct_def = &self.cx[ct];
match ct_def.ctor {
match ct_def.kind {
// HACK(eddyb) don't push an `UseOrigin` for `GlobalVar` pointers.
ConstCtor::PtrToGlobalVar(_) if ct_def.attrs == AttrSet::default() => {
ConstKind::PtrToGlobalVar(_) if ct_def.attrs == AttrSet::default() => {
self.visit_const_def(ct_def);
}
_ => {
Expand Down Expand Up @@ -642,12 +645,12 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> {
// Treat this like a call, in the caller.
replace_origin(self, IntraFuncUseOrigin::CallCallee);

let const_ctor = |v: Value| match v {
Value::Const(ct) => &self.cx[ct].ctor,
let const_kind = |v: Value| match v {
Value::Const(ct) => &self.cx[ct].kind,
_ => unreachable!(),
};
let const_str = |v: Value| match const_ctor(v) {
&ConstCtor::SpvStringLiteralForExtInst(s) => s,
let const_str = |v: Value| match const_kind(v) {
&ConstKind::SpvStringLiteralForExtInst(s) => s,
_ => unreachable!(),
};
self.use_stack.push(UseOrigin::IntraFunc {
Expand Down
Loading