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.

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

Add debugPrintf-based panic reporting, controlled via spirv_builder::ShaderPanicStrategy. #1080

Merged
merged 1 commit into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added ⭐
- [PR#1080](https://github.com/EmbarkStudios/rust-gpu/pull/1080) added `debugPrintf`-based
panic reporting, with the desired behavior selected via `spirv_builder::ShaderPanicStrategy`
(see its documentation for more details about each available panic handling strategy)

### Changed 🛠
- [PR#1079](https://github.com/EmbarkStudios/rust-gpu/pull/1079) revised `spirv-builder`'s `README.md`,
and added a way for `docs.rs` to be able to build it (via `cargo +stable doc --no-default-features`)
Expand Down
5 changes: 3 additions & 2 deletions crates/rustc_codegen_spirv/src/builder/builder_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_codegen_ssa::common::{
use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
use rustc_codegen_ssa::mir::place::PlaceRef;
use rustc_codegen_ssa::traits::{
BackendTypes, BuilderMethods, ConstMethods, IntrinsicCallMethods, LayoutTypeMethods, OverflowOp,
BackendTypes, BuilderMethods, ConstMethods, LayoutTypeMethods, OverflowOp,
};
use rustc_codegen_ssa::MemFlags;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -2647,7 +2647,8 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {

// HACK(eddyb) redirect any possible panic call to an abort, to avoid
// needing to materialize `&core::panic::Location` or `format_args!`.
self.abort();
// FIXME(eddyb) find a way to extract the original message.
self.abort_with_message("panic!(...)".into());
self.undef(result_type)
} else if let Some(mode) = buffer_load_intrinsic {
self.codegen_buffer_load_intrinsic(result_type, args, mode)
Expand Down
37 changes: 25 additions & 12 deletions crates/rustc_codegen_spirv/src/builder/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::builder_spirv::{SpirvValue, SpirvValueExt};
use crate::codegen_cx::CodegenCx;
use crate::custom_insts::CustomInst;
use crate::spirv_type::SpirvType;
use rspirv::dr::Operand;
use rspirv::spirv::GLOp;
use rustc_codegen_ssa::mir::operand::OperandRef;
use rustc_codegen_ssa::mir::place::PlaceRef;
Expand Down Expand Up @@ -338,18 +339,7 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
}

fn abort(&mut self) {
// FIXME(eddyb) this should be cached more efficiently.
let void_ty = SpirvType::Void.def(rustc_span::DUMMY_SP, self);

// HACK(eddyb) there is no `abort` or `trap` instruction in SPIR-V,
// so the best thing we can do is use our own custom instruction.
self.custom_inst(void_ty, CustomInst::Abort);
self.unreachable();

// HACK(eddyb) we still need an active block in case the user of this
// `Builder` will continue to emit instructions after the `.abort()`.
let post_abort_dead_bb = self.append_sibling_block("post_abort_dead");
self.switch_to_block(post_abort_dead_bb);
self.abort_with_message("intrinsics::abort()".into());
}

fn assume(&mut self, _val: Self::Value) {
Expand Down Expand Up @@ -382,3 +372,26 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
todo!()
}
}

impl Builder<'_, '_> {
pub fn abort_with_message(&mut self, message: String) {
// FIXME(eddyb) this should be cached more efficiently.
let void_ty = SpirvType::Void.def(rustc_span::DUMMY_SP, self);

// HACK(eddyb) there is no `abort` or `trap` instruction in SPIR-V,
// so the best thing we can do is use our own custom instruction.
let message_id = self.emit().string(message);
self.custom_inst(
void_ty,
CustomInst::Abort {
message: Operand::IdRef(message_id),
},
);
self.unreachable();

// HACK(eddyb) we still need an active block in case the user of this
// `Builder` will continue to emit instructions after the `.abort()`.
let post_abort_dead_bb = self.append_sibling_block("post_abort_dead");
self.switch_to_block(post_abort_dead_bb);
}
}
8 changes: 8 additions & 0 deletions crates/rustc_codegen_spirv/src/codegen_cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,12 @@ impl CodegenArgs {
"enable additional SPIR-T passes (comma-separated)",
"PASSES",
);
opts.optopt(
"",
"abort-strategy",
"select a non-default abort (i.e. panic) strategy - see `spirv-builder` docs",
"STRATEGY",
);

// NOTE(eddyb) these are debugging options that used to be env vars
// (for more information see `docs/src/codegen-args.md`).
Expand Down Expand Up @@ -529,6 +535,8 @@ impl CodegenArgs {
.map(|s| s.to_string())
.collect(),

abort_strategy: matches.opt_str("abort-strategy"),

// FIXME(eddyb) deduplicate between `CodegenArgs` and `linker::Options`.
emit_multiple_modules: module_output_type == ModuleOutputType::Multiple,
spirv_metadata,
Expand Down
8 changes: 4 additions & 4 deletions crates/rustc_codegen_spirv/src/custom_insts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,15 @@ def_custom_insts! {
// an additional returned `bool`, instead (i.e. a form of emulated unwinding).
//
// As this is a custom terminator, it must only appear before `OpUnreachable`,
// *without* any instructions in between (not even debuginfo ones).
// with at most debuginfo instructions (standard or custom), between the two.
//
// FIXME(eddyb) long-term this kind of custom control-flow could be generalized
// to fully emulate unwinding (resulting in codegen similar to `?` in functions
// returning `Option` or `Result`), to e.g. run destructors, or even allow
// users to do `catch_unwind` at the top-level of their shader to handle
// panics specially (e.g. by appending to a custom buffer, or using some
// specific color in a fragment shader, to indicate a panic happened).
4 => Abort,
4 => Abort { message },
}

impl CustomOp {
Expand All @@ -164,8 +164,8 @@ impl CustomOp {
}

/// Returns `true` iff this `CustomOp` is a custom terminator instruction,
/// i.e. semantic and must always appear just before an `OpUnreachable`
/// standard terminator (without even debuginfo in between the two).
/// i.e. semantic and must precede an `OpUnreachable` standard terminator,
/// with at most debuginfo instructions (standard or custom), between the two.
pub fn is_terminator(self) -> bool {
match self {
CustomOp::SetDebugSrcLoc
Expand Down
Loading
Loading