Skip to content

Commit 1abd1cf

Browse files
eddybeddyb
committed
Show panic! messages via debugPrintf, even including some runtime arguments ({u,i,f}32 as {} or {:?}).
1 parent a0347e2 commit 1abd1cf

File tree

6 files changed

+201
-31
lines changed

6 files changed

+201
-31
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3030
## [Unreleased]
3131

3232
### Added ⭐
33+
- [PR#1082](https://github.com/EmbarkStudios/rust-gpu/pull/1082) added partial
34+
support for extracting `format_args!` from `panic!`s, and converting them to
35+
`debugPrintf` calls (if enabled via `ShaderPanicStrategy`), including runtime
36+
arguments (`u32`/`i32`/`f32` with `Display`/`Debug` formatting, for now)
3337
- [PR#1081](https://github.com/EmbarkStudios/rust-gpu/pull/1081) added the ability
3438
to access SPIR-V specialization constants (`OpSpecConstant`) via entry-point
3539
inputs declared as `#[spirv(spec_constant(id = ..., default = ...))] x: u32`

‎crates/rustc_codegen_spirv/src/builder/builder_methods.rs

Lines changed: 137 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ use rustc_codegen_ssa::MemFlags;
2020
use rustc_data_structures::fx::FxHashSet;
2121
use rustc_middle::bug;
2222
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
23+
use rustc_middle::ty::layout::LayoutOf;
2324
use rustc_middle::ty::Ty;
2425
use rustc_span::Span;
2526
use rustc_target::abi::call::FnAbi;
2627
use rustc_target::abi::{Abi, Align, Scalar, Size, WrappingRange};
2728
use smallvec::SmallVec;
29+
use std::borrow::Cow;
2830
use std::cell::Cell;
2931
use std::convert::TryInto;
3032
use std::iter::{self, empty};
@@ -2414,12 +2416,25 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
24142416
// nor simplified in MIR (e.g. promoted to a constant) in any way,
24152417
// so we have to try and remove the `fmt::Arguments::new` call here.
24162418
#[derive(Default)]
2417-
struct DecodedFormatArgs {}
2419+
struct DecodedFormatArgs<'tcx> {
2420+
/// If fully constant, the `pieces: &'a [&'static str]` input
2421+
/// of `fmt::Arguments<'a>` (i.e. the strings between args).
2422+
const_pieces: Option<SmallVec<[String; 2]>>,
2423+
2424+
/// Original references for `fmt::Arguments<'a>` dynamic arguments,
2425+
/// i.e. the `&'a T` passed to `fmt::rt::Argument::<'a>::new_*`,
2426+
/// tracking the type `T` and `char` formatting specifier.
2427+
///
2428+
/// E.g. for `format_args!("{a} {b:x}")` they'll be:
2429+
/// * `&a` with `typeof a` and ' ',
2430+
/// *`&b` with `typeof b` and 'x'
2431+
ref_arg_ids_with_ty_and_spec: SmallVec<[(Word, Ty<'tcx>, char); 2]>,
2432+
}
24182433
struct FormatArgsNotRecognized(String);
24192434

24202435
// HACK(eddyb) this is basically a `try` block.
24212436
let try_decode_and_remove_format_args = || {
2422-
let decoded_format_args = DecodedFormatArgs::default();
2437+
let mut decoded_format_args = DecodedFormatArgs::default();
24232438

24242439
let const_u32_as_usize = |ct_id| match self.builder.lookup_const_by_id(ct_id)? {
24252440
SpirvConst::U32(x) => Some(x as usize),
@@ -2441,6 +2456,32 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
24412456
}
24422457
None
24432458
};
2459+
let const_str_as_utf8 = |str_ptr_and_len_ids: &[Word]| {
2460+
let piece_str_bytes = const_slice_as_elem_ids(str_ptr_and_len_ids)?
2461+
.iter()
2462+
.map(|&id| u8::try_from(const_u32_as_usize(id)?).ok())
2463+
.collect::<Option<Vec<u8>>>()?;
2464+
String::from_utf8(piece_str_bytes).ok()
2465+
};
2466+
2467+
// HACK(eddyb) some entry-points only take a `&str`, not `fmt::Arguments`.
2468+
if let [
2469+
SpirvValue {
2470+
kind: SpirvValueKind::Def(a_id),
2471+
..
2472+
},
2473+
SpirvValue {
2474+
kind: SpirvValueKind::Def(b_id),
2475+
..
2476+
},
2477+
_, // `&'static panic::Location<'static>`
2478+
] = args[..]
2479+
{
2480+
if let Some(const_msg) = const_str_as_utf8(&[a_id, b_id]) {
2481+
decoded_format_args.const_pieces = Some([const_msg].into_iter().collect());
2482+
return Ok(decoded_format_args);
2483+
}
2484+
}
24442485

24452486
let format_args_id = match args {
24462487
&[
@@ -2503,6 +2544,7 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
25032544
#[derive(Debug)]
25042545
enum Inst<'tcx, ID> {
25052546
Bitcast(ID, ID),
2547+
CompositeExtract(ID, ID, u32),
25062548
AccessChain(ID, ID, SpirvConst<'tcx>),
25072549
InBoundsAccessChain(ID, ID, SpirvConst<'tcx>),
25082550
Store(ID, ID),
@@ -2519,6 +2561,15 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
25192561
let (i, inst) = non_debug_insts.next_back()?;
25202562
taken_inst_idx_range.start.set(i);
25212563

2564+
// HACK(eddyb) avoid the logic below that assumes only ID operands
2565+
if inst.class.opcode == Op::CompositeExtract {
2566+
if let (Some(r), &[Operand::IdRef(x), Operand::LiteralInt32(i)]) =
2567+
(inst.result_id, &inst.operands[..])
2568+
{
2569+
return Some(Inst::CompositeExtract(r, x, i));
2570+
}
2571+
}
2572+
25222573
// HACK(eddyb) all instructions accepted below
25232574
// are expected to take no more than 4 operands,
25242575
// and this is easier to use than an iterator.
@@ -2716,6 +2767,26 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
27162767
}
27172768
}
27182769

2770+
// If the `pieces: &[&str]` slice needs a bitcast, it'll be here.
2771+
let pieces_slice_ptr_id = match try_rev_take(1).as_deref() {
2772+
Some(&[Inst::Bitcast(out_id, in_id)]) if out_id == pieces_slice_ptr_id => in_id,
2773+
_ => pieces_slice_ptr_id,
2774+
};
2775+
decoded_format_args.const_pieces =
2776+
const_slice_as_elem_ids(&[pieces_slice_ptr_id, pieces_len_id]).and_then(
2777+
|piece_ids| {
2778+
piece_ids
2779+
.iter()
2780+
.map(|&piece_id| {
2781+
match self.builder.lookup_const_by_id(piece_id)? {
2782+
SpirvConst::Composite(piece) => const_str_as_utf8(piece),
2783+
_ => None,
2784+
}
2785+
})
2786+
.collect::<Option<_>>()
2787+
},
2788+
);
2789+
27192790
// Keep all instructions up to (but not including) the last one
27202791
// confirmed above to be the first instruction of `format_args!`.
27212792
func.blocks[block_idx]
@@ -2725,8 +2796,61 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
27252796
Ok(decoded_format_args)
27262797
};
27272798

2728-
match try_decode_and_remove_format_args() {
2729-
Ok(DecodedFormatArgs {}) => {}
2799+
let mut debug_printf_args = SmallVec::<[_; 2]>::new();
2800+
let message = match try_decode_and_remove_format_args() {
2801+
Ok(DecodedFormatArgs {
2802+
const_pieces,
2803+
ref_arg_ids_with_ty_and_spec,
2804+
}) => {
2805+
match const_pieces {
2806+
Some(const_pieces) => {
2807+
const_pieces
2808+
.into_iter()
2809+
.map(|s| Cow::Owned(s.replace('%', "%%")))
2810+
.interleave(ref_arg_ids_with_ty_and_spec.iter().map(
2811+
|&(ref_id, ty, spec)| {
2812+
use rustc_target::abi::{Integer::*, Primitive::*};
2813+
2814+
let layout = self.layout_of(ty);
2815+
2816+
let scalar = match layout.abi {
2817+
Abi::Scalar(scalar) => Some(scalar.primitive()),
2818+
_ => None,
2819+
};
2820+
let debug_printf_fmt = match (spec, scalar) {
2821+
// FIXME(eddyb) support more of these,
2822+
// potentially recursing to print ADTs.
2823+
(' ' | '?', Some(Int(I32, false))) => "%u",
2824+
('x', Some(Int(I32, false))) => "%x",
2825+
(' ' | '?', Some(Int(I32, true))) => "%i",
2826+
(' ' | '?', Some(F32)) => "%f",
2827+
2828+
_ => "",
2829+
};
2830+
2831+
if debug_printf_fmt.is_empty() {
2832+
return Cow::Owned(
2833+
format!("{{/* unprintable {ty} */:{spec}}}")
2834+
.replace('%', "%%"),
2835+
);
2836+
}
2837+
2838+
let spirv_type = layout.spirv_type(self.span(), self);
2839+
debug_printf_args.push(
2840+
self.emit()
2841+
.load(spirv_type, None, ref_id, None, [])
2842+
.unwrap()
2843+
.with_type(spirv_type),
2844+
);
2845+
Cow::Borrowed(debug_printf_fmt)
2846+
},
2847+
))
2848+
.collect::<String>()
2849+
}
2850+
None => "<unknown message>".into(),
2851+
}
2852+
}
2853+
27302854
Err(FormatArgsNotRecognized(step)) => {
27312855
if let Some(current_span) = self.current_span {
27322856
let mut warn = self.tcx.sess.struct_span_warn(
@@ -2738,8 +2862,8 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
27382862
"compilation may later fail due to leftover `format_args!` internals",
27392863
);
27402864

2741-
if self.tcx.sess.opts.unstable_opts.inline_mir != Some(true) {
2742-
warn.note("missing `-Zinline-mir=on` flag (should've been set by `spirv-builder`)")
2865+
if self.tcx.sess.opts.unstable_opts.inline_mir != Some(false) {
2866+
warn.note("missing `-Zinline-mir=off` flag (should've been set by `spirv-builder`)")
27432867
.help("check `.cargo` and environment variables for potential overrides")
27442868
.help("(or, if not using `spirv-builder` at all, add the flag manually)");
27452869
} else {
@@ -2748,13 +2872,17 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
27482872

27492873
warn.emit();
27502874
}
2875+
"<unknown message> (failed to find/decode `format_args!` expansion)".into()
27512876
}
2752-
}
2877+
};
27532878

27542879
// HACK(eddyb) redirect any possible panic call to an abort, to avoid
27552880
// needing to materialize `&core::panic::Location` or `format_args!`.
2756-
// FIXME(eddyb) find a way to extract the original message.
2757-
self.abort_with_message("panicked: <unknown message>".into());
2881+
self.abort_with_message_and_debug_printf_args(
2882+
// HACK(eddyb) `|` is an ad-hoc convention of `linker::spirt_passes::controlflow`.
2883+
format!("panicked|{message}"),
2884+
debug_printf_args,
2885+
);
27582886
self.undef(result_type)
27592887
} else if let Some(mode) = buffer_load_intrinsic {
27602888
self.codegen_buffer_load_intrinsic(result_type, args, mode)

‎crates/rustc_codegen_spirv/src/builder/intrinsics.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,11 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
339339
}
340340

341341
fn abort(&mut self) {
342-
self.abort_with_message("aborted: intrinsics::abort() called".into());
342+
// HACK(eddyb) `|` is an ad-hoc convention of `linker::spirt_passes::controlflow`.
343+
self.abort_with_message_and_debug_printf_args(
344+
"aborted|intrinsics::abort() called".into(),
345+
[],
346+
);
343347
}
344348

345349
fn assume(&mut self, _val: Self::Value) {
@@ -374,7 +378,11 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
374378
}
375379

376380
impl Builder<'_, '_> {
377-
pub fn abort_with_message(&mut self, message: String) {
381+
pub fn abort_with_message_and_debug_printf_args(
382+
&mut self,
383+
message: String,
384+
debug_printf_args: impl IntoIterator<Item = SpirvValue>,
385+
) {
378386
// FIXME(eddyb) this should be cached more efficiently.
379387
let void_ty = SpirvType::Void.def(rustc_span::DUMMY_SP, self);
380388

@@ -385,6 +393,10 @@ impl Builder<'_, '_> {
385393
void_ty,
386394
CustomInst::Abort {
387395
message: Operand::IdRef(message_id),
396+
debug_printf_args: debug_printf_args
397+
.into_iter()
398+
.map(|arg| Operand::IdRef(arg.def(self)))
399+
.collect(),
388400
},
389401
);
390402
self.unreachable();

‎crates/rustc_codegen_spirv/src/codegen_cx/constant.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,16 @@ impl<'tcx> ConstMethods<'tcx> for CodegenCx<'tcx> {
172172
let str_ty = self
173173
.layout_of(self.tcx.types.str_)
174174
.spirv_type(DUMMY_SP, self);
175-
// FIXME(eddyb) include the actual byte data.
176175
(
177176
self.def_constant(
178177
self.type_ptr_to(str_ty),
179178
SpirvConst::PtrTo {
180-
pointee: self.undef(str_ty).def_cx(self),
179+
pointee: self
180+
.constant_composite(
181+
str_ty,
182+
s.bytes().map(|b| self.const_u8(b).def_cx(self)),
183+
)
184+
.def_cx(self),
181185
},
182186
),
183187
self.const_usize(len as u64),

‎crates/rustc_codegen_spirv/src/custom_insts.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ lazy_static! {
4949
}
5050

5151
macro_rules! def_custom_insts {
52-
($($num:literal => $name:ident $({ $($field:ident),+ $(,)? })?),+ $(,)?) => {
52+
($($num:literal => $name:ident $({ $($field:ident),+ $(, ..$variadic_field:ident)? $(,)? })?),+ $(,)?) => {
5353
const SCHEMA: &[(u32, &str, &[&str])] = &[
54-
$(($num, stringify!($name), &[$($(stringify!($field)),+)?])),+
54+
$(($num, stringify!($name), &[$($(stringify!($field),)+ $(stringify!(..$variadic_field),)?)?])),+
5555
];
5656

5757
#[repr(u32)]
@@ -74,16 +74,19 @@ macro_rules! def_custom_insts {
7474
pub fn with_operands<T: Clone>(self, operands: &[T]) -> CustomInst<T> {
7575
match self {
7676
$(Self::$name => match operands {
77-
[$($($field),+)?] => CustomInst::$name $({ $($field: $field.clone()),+ })?,
77+
[$($($field,)+ $(ref $variadic_field @ ..)?)?] => CustomInst::$name $({
78+
$($field: $field.clone(),)+
79+
$($variadic_field: $variadic_field.iter().cloned().collect())?
80+
})?,
7881
_ => unreachable!("{self:?} does not have the right number of operands"),
7982
}),+
8083
}
8184
}
8285
}
8386

84-
#[derive(Copy, Clone, Debug)]
87+
#[derive(Clone, Debug)]
8588
pub enum CustomInst<T> {
86-
$($name $({ $($field: T),+ })?),+
89+
$($name $({ $($field: T,)+ $($variadic_field: SmallVec<[T; 4]>)? })?),+
8790
}
8891

8992
impl<T> CustomInst<T> {
@@ -96,7 +99,9 @@ macro_rules! def_custom_insts {
9699
// HACK(eddyb) this should return an iterator, but that's too much effort.
97100
pub fn into_operands(self) -> SmallVec<[T; 8]> {
98101
match self {
99-
$(Self::$name $({ $($field),+ })? => [$($($field),+)?].into_iter().collect()),+
102+
$(Self::$name $({ $($field,)+ $($variadic_field)? })? => {
103+
[$($($field),+)?].into_iter() $($(.chain($variadic_field))?)? .collect()
104+
})+
100105
}
101106
}
102107
}
@@ -146,7 +151,9 @@ def_custom_insts! {
146151
// users to do `catch_unwind` at the top-level of their shader to handle
147152
// panics specially (e.g. by appending to a custom buffer, or using some
148153
// specific color in a fragment shader, to indicate a panic happened).
149-
4 => Abort { message },
154+
// NOTE(eddyb) the `message` string follows `debugPrintf` rules, with remaining
155+
// operands (collected into `debug_printf_args`) being its formatting arguments.
156+
4 => Abort { message, ..debug_printf_args },
150157
}
151158

152159
impl CustomOp {

0 commit comments

Comments
 (0)