Skip to content

Commit a0347e2

Browse files
eddybeddyb
committed
Allow -C debuginfo=2, but require -Zinline-mir=off, for panic! format_args! removal.
1 parent 4c6cf0d commit a0347e2

File tree

5 files changed

+246
-104
lines changed

5 files changed

+246
-104
lines changed

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

Lines changed: 198 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::builder_spirv::{BuilderCursor, SpirvConst, SpirvValue, SpirvValueExt,
44
use crate::custom_insts::{CustomInst, CustomOp};
55
use crate::rustc_codegen_ssa::traits::BaseTypeMethods;
66
use crate::spirv_type::SpirvType;
7+
use itertools::Itertools;
78
use rspirv::dr::{InsertPoint, Instruction, Operand};
89
use rspirv::spirv::{Capability, MemoryModel, MemorySemantics, Op, Scope, StorageClass, Word};
910
use rustc_apfloat::{ieee, Float, Round, Status};
@@ -24,6 +25,7 @@ use rustc_span::Span;
2425
use rustc_target::abi::call::FnAbi;
2526
use rustc_target::abi::{Abi, Align, Scalar, Size, WrappingRange};
2627
use smallvec::SmallVec;
28+
use std::cell::Cell;
2729
use std::convert::TryInto;
2830
use std::iter::{self, empty};
2931

@@ -2411,18 +2413,49 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
24112413
// more complex and neither immediate (`fmt::Arguments` is too big)
24122414
// nor simplified in MIR (e.g. promoted to a constant) in any way,
24132415
// so we have to try and remove the `fmt::Arguments::new` call here.
2416+
#[derive(Default)]
2417+
struct DecodedFormatArgs {}
2418+
struct FormatArgsNotRecognized(String);
2419+
24142420
// HACK(eddyb) this is basically a `try` block.
2415-
let remove_format_args_if_possible = || -> Option<()> {
2421+
let try_decode_and_remove_format_args = || {
2422+
let decoded_format_args = DecodedFormatArgs::default();
2423+
2424+
let const_u32_as_usize = |ct_id| match self.builder.lookup_const_by_id(ct_id)? {
2425+
SpirvConst::U32(x) => Some(x as usize),
2426+
_ => None,
2427+
};
2428+
let const_slice_as_elem_ids = |slice_ptr_and_len_ids: &[Word]| {
2429+
if let [ptr_id, len_id] = slice_ptr_and_len_ids[..] {
2430+
if let SpirvConst::PtrTo { pointee } =
2431+
self.builder.lookup_const_by_id(ptr_id)?
2432+
{
2433+
if let SpirvConst::Composite(elems) =
2434+
self.builder.lookup_const_by_id(pointee)?
2435+
{
2436+
if elems.len() == const_u32_as_usize(len_id)? {
2437+
return Some(elems);
2438+
}
2439+
}
2440+
}
2441+
}
2442+
None
2443+
};
2444+
24162445
let format_args_id = match args {
24172446
&[
24182447
SpirvValue {
24192448
kind: SpirvValueKind::Def(format_args_id),
24202449
..
24212450
},
2422-
_,
2451+
_, // `&'static panic::Location<'static>`
24232452
] => format_args_id,
24242453

2425-
_ => return None,
2454+
_ => {
2455+
return Err(FormatArgsNotRecognized(
2456+
"panic entry-point call args".into(),
2457+
));
2458+
}
24262459
};
24272460

24282461
let custom_ext_inst_set_import = self.ext_inst.borrow_mut().import_custom(self);
@@ -2446,8 +2479,11 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
24462479
.take_while(|inst| inst.class.opcode == Op::Variable)
24472480
.map(|inst| inst.result_id.unwrap())
24482481
.collect();
2449-
let require_local_var =
2450-
|ptr_id| Some(()).filter(|()| local_var_ids.contains(&ptr_id));
2482+
let require_local_var = |ptr_id, var| {
2483+
Some(())
2484+
.filter(|()| local_var_ids.contains(&ptr_id))
2485+
.ok_or_else(|| FormatArgsNotRecognized(format!("{var} storage not local")))
2486+
};
24512487

24522488
let mut non_debug_insts = func.blocks[block_idx]
24532489
.instructions
@@ -2474,14 +2510,14 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
24742510
Call(ID, ID, SmallVec<[ID; 4]>),
24752511
}
24762512

2477-
let mut taken_inst_idx_range = func.blocks[block_idx].instructions.len()..;
2513+
let taken_inst_idx_range = Cell::new(func.blocks[block_idx].instructions.len())..;
24782514

24792515
// Take `count` instructions, advancing backwards, but returning
24802516
// instructions in their original order (and decoded to `Inst`s).
24812517
let mut try_rev_take = |count| {
24822518
let maybe_rev_insts = (0..count).map(|_| {
24832519
let (i, inst) = non_debug_insts.next_back()?;
2484-
taken_inst_idx_range = i..;
2520+
taken_inst_idx_range.start.set(i);
24852521

24862522
// HACK(eddyb) all instructions accepted below
24872523
// are expected to take no more than 4 operands,
@@ -2519,102 +2555,143 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
25192555
insts.reverse();
25202556
Some(insts)
25212557
};
2558+
let fmt_args_new_call_insts = try_rev_take(3).ok_or_else(|| {
2559+
FormatArgsNotRecognized(
2560+
"fmt::Arguments::new call: ran out of instructions".into(),
2561+
)
2562+
})?;
2563+
let ((pieces_slice_ptr_id, pieces_len_id), (rt_args_slice_ptr_id, rt_args_count)) =
2564+
match fmt_args_new_call_insts[..] {
2565+
[
2566+
Inst::Call(call_ret_id, callee_id, ref call_args),
2567+
Inst::Store(st_dst_id, st_val_id),
2568+
Inst::Load(ld_val_id, ld_src_id),
2569+
] if self.fmt_args_new_fn_ids.borrow().contains(&callee_id)
2570+
&& call_ret_id == st_val_id
2571+
&& st_dst_id == ld_src_id
2572+
&& ld_val_id == format_args_id =>
2573+
{
2574+
require_local_var(st_dst_id, "fmt::Arguments::new destination")?;
2575+
2576+
match call_args[..] {
2577+
// `<core::fmt::Arguments>::new_v1`
2578+
[
2579+
pieces_slice_ptr_id,
2580+
pieces_len_id,
2581+
rt_args_slice_ptr_id,
2582+
rt_args_len_id,
2583+
] => (
2584+
(pieces_slice_ptr_id, pieces_len_id),
2585+
(
2586+
Some(rt_args_slice_ptr_id),
2587+
const_u32_as_usize(rt_args_len_id).ok_or_else(|| {
2588+
FormatArgsNotRecognized(
2589+
"fmt::Arguments::new: args.len() not constant"
2590+
.into(),
2591+
)
2592+
})?,
2593+
),
2594+
),
25222595

2523-
let (rt_args_slice_ptr_id, rt_args_count) = match try_rev_take(3)?[..] {
2524-
[
2525-
Inst::Call(call_ret_id, callee_id, ref call_args),
2526-
Inst::Store(st_dst_id, st_val_id),
2527-
Inst::Load(ld_val_id, ld_src_id),
2528-
] if self.fmt_args_new_fn_ids.borrow().contains(&callee_id)
2529-
&& call_ret_id == st_val_id
2530-
&& st_dst_id == ld_src_id
2531-
&& ld_val_id == format_args_id =>
2532-
{
2533-
require_local_var(st_dst_id)?;
2534-
match call_args[..] {
2535-
// `<core::fmt::Arguments>::new_v1`
2536-
[_, _, rt_args_slice_ptr_id, rt_args_len_id] => (
2537-
Some(rt_args_slice_ptr_id),
2538-
self.builder
2539-
.lookup_const_by_id(rt_args_len_id)
2540-
.and_then(|ct| match ct {
2541-
SpirvConst::U32(x) => Some(x as usize),
2542-
_ => None,
2543-
})?,
2544-
),
2545-
2546-
// `<core::fmt::Arguments>::new_const`
2547-
[_, _] => (None, 0),
2548-
2549-
_ => return None,
2596+
// `<core::fmt::Arguments>::new_const`
2597+
[pieces_slice_ptr_id, pieces_len_id] => {
2598+
((pieces_slice_ptr_id, pieces_len_id), (None, 0))
2599+
}
2600+
2601+
_ => {
2602+
return Err(FormatArgsNotRecognized(
2603+
"fmt::Arguments::new call args".into(),
2604+
));
2605+
}
2606+
}
25502607
}
2551-
}
2552-
_ => return None,
2553-
};
2608+
_ => {
2609+
// HACK(eddyb) this gathers more context before reporting.
2610+
let mut insts = fmt_args_new_call_insts;
2611+
insts.reverse();
2612+
while let Some(extra_inst) = try_rev_take(1) {
2613+
insts.extend(extra_inst);
2614+
if insts.len() >= 32 {
2615+
break;
2616+
}
2617+
}
2618+
insts.reverse();
2619+
2620+
return Err(FormatArgsNotRecognized(format!(
2621+
"fmt::Arguments::new call sequence ({insts:?})",
2622+
)));
2623+
}
2624+
};
25542625

25552626
// HACK(eddyb) this is the worst part: if we do have runtime
25562627
// arguments (from e.g. new `assert!`s being added to `core`),
25572628
// we have to confirm their many instructions for removal.
25582629
if rt_args_count > 0 {
25592630
let rt_args_slice_ptr_id = rt_args_slice_ptr_id.unwrap();
2560-
let rt_args_array_ptr_id = match try_rev_take(1)?[..] {
2631+
let rt_args_array_ptr_id = match try_rev_take(1).ok_or_else(|| {
2632+
FormatArgsNotRecognized(
2633+
"&[fmt::rt::Argument] bitcast: ran out of instructions".into(),
2634+
)
2635+
})?[..]
2636+
{
25612637
[Inst::Bitcast(out_id, in_id)] if out_id == rt_args_slice_ptr_id => in_id,
2562-
_ => return None,
2638+
_ => {
2639+
return Err(FormatArgsNotRecognized(
2640+
"&[fmt::rt::Argument] bitcast".into(),
2641+
));
2642+
}
25632643
};
2564-
require_local_var(rt_args_array_ptr_id);
2644+
require_local_var(rt_args_array_ptr_id, "[fmt::rt::Argument; N]")?;
25652645

2566-
// Each runtime argument has its own variable, 6 instructions
2567-
// to initialize it, and 9 instructions to copy it to the
2568-
// appropriate slot in the array. The groups of 6 and 9
2646+
// Each runtime argument has 3 instructions to call one of
2647+
// the `fmt::rt::Argument::new_*` functions (and split its
2648+
// scalar pair result), and 5 instructions to store it into
2649+
// the appropriate slot in the array. The groups of 3 and 5
25692650
// instructions, for all runtime args, are each separate.
2570-
let copies_from_rt_arg_vars_to_rt_args_array = try_rev_take(rt_args_count * 9)?;
2571-
let copies_from_rt_arg_vars_to_rt_args_array =
2572-
copies_from_rt_arg_vars_to_rt_args_array.chunks(9);
2573-
let inits_of_rt_arg_vars = try_rev_take(rt_args_count * 6)?;
2574-
let inits_of_rt_arg_vars = inits_of_rt_arg_vars.chunks(6);
2575-
2576-
for (
2577-
rt_arg_idx,
2578-
(init_of_rt_arg_var_insts, copy_from_rt_arg_var_to_rt_args_array_insts),
2579-
) in inits_of_rt_arg_vars
2580-
.zip(copies_from_rt_arg_vars_to_rt_args_array)
2581-
.enumerate()
2651+
let stores_to_rt_args_array =
2652+
try_rev_take(rt_args_count * 5).ok_or_else(|| {
2653+
FormatArgsNotRecognized(
2654+
"[fmt::rt::Argument; N] stores: ran out of instructions".into(),
2655+
)
2656+
})?;
2657+
let stores_to_rt_args_array = stores_to_rt_args_array.chunks(5);
2658+
let rt_arg_new_calls = try_rev_take(rt_args_count * 3).ok_or_else(|| {
2659+
FormatArgsNotRecognized(
2660+
"fmt::rt::Argument::new calls: ran out of instructions".into(),
2661+
)
2662+
})?;
2663+
let rt_arg_new_calls = rt_arg_new_calls.chunks(3);
2664+
2665+
for (rt_arg_idx, (rt_arg_new_call_insts, store_to_rt_args_array_insts)) in
2666+
rt_arg_new_calls.zip(stores_to_rt_args_array).enumerate()
25822667
{
2583-
let rt_arg_var_id = match init_of_rt_arg_var_insts[..] {
2668+
let (a, b) = match rt_arg_new_call_insts[..] {
25842669
[
2585-
Inst::Bitcast(b, _),
2586-
Inst::Bitcast(a, _),
2587-
Inst::AccessChain(a_ptr, a_base_ptr, SpirvConst::U32(0)),
2588-
Inst::Store(a_st_dst, a_st_val),
2589-
Inst::AccessChain(b_ptr, b_base_ptr, SpirvConst::U32(1)),
2590-
Inst::Store(b_st_dst, b_st_val),
2591-
] if a_base_ptr == b_base_ptr
2592-
&& (a, b) == (a_st_val, b_st_val)
2593-
&& (a_ptr, b_ptr) == (a_st_dst, b_st_dst) =>
2594-
{
2595-
require_local_var(a_base_ptr);
2596-
a_base_ptr
2597-
}
2598-
_ => return None,
2599-
};
2670+
Inst::Call(call_ret_id, callee_id, ref call_args),
2671+
Inst::CompositeExtract(a, a_parent_pair, 0),
2672+
Inst::CompositeExtract(b, b_parent_pair, 1),
2673+
] if [a_parent_pair, b_parent_pair] == [call_ret_id; 2] => self
2674+
.fmt_rt_arg_new_fn_ids_to_ty_and_spec
2675+
.borrow()
2676+
.get(&callee_id)
2677+
.and_then(|&(ty, spec)| match call_args[..] {
2678+
[x] => {
2679+
decoded_format_args
2680+
.ref_arg_ids_with_ty_and_spec
2681+
.push((x, ty, spec));
2682+
Some((a, b))
2683+
}
2684+
_ => None,
2685+
}),
2686+
_ => None,
2687+
}
2688+
.ok_or_else(|| {
2689+
FormatArgsNotRecognized(format!(
2690+
"fmt::rt::Argument::new call sequence ({rt_arg_new_call_insts:?})"
2691+
))
2692+
})?;
26002693

2601-
// HACK(eddyb) this is only split to allow variable name reuse.
2602-
let (copy_loads, copy_stores) =
2603-
copy_from_rt_arg_var_to_rt_args_array_insts.split_at(4);
2604-
let (a, b) = match copy_loads[..] {
2605-
[
2606-
Inst::AccessChain(a_ptr, a_base_ptr, SpirvConst::U32(0)),
2607-
Inst::Load(a_ld_val, a_ld_src),
2608-
Inst::AccessChain(b_ptr, b_base_ptr, SpirvConst::U32(1)),
2609-
Inst::Load(b_ld_val, b_ld_src),
2610-
] if [a_base_ptr, b_base_ptr] == [rt_arg_var_id; 2]
2611-
&& (a_ptr, b_ptr) == (a_ld_src, b_ld_src) =>
2612-
{
2613-
(a_ld_val, b_ld_val)
2614-
}
2615-
_ => return None,
2616-
};
2617-
match copy_stores[..] {
2694+
match store_to_rt_args_array_insts[..] {
26182695
[
26192696
Inst::InBoundsAccessChain(
26202697
array_slot_ptr,
@@ -2630,7 +2707,11 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
26302707
&& [a_base_ptr, b_base_ptr] == [array_slot_ptr; 2]
26312708
&& (a, b) == (a_st_val, b_st_val)
26322709
&& (a_ptr, b_ptr) == (a_st_dst, b_st_dst) => {}
2633-
_ => return None,
2710+
_ => {
2711+
return Err(FormatArgsNotRecognized(format!(
2712+
"[fmt::rt::Argument; N] stores sequence ({store_to_rt_args_array_insts:?})"
2713+
)));
2714+
}
26342715
}
26352716
}
26362717
}
@@ -2639,11 +2720,36 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
26392720
// confirmed above to be the first instruction of `format_args!`.
26402721
func.blocks[block_idx]
26412722
.instructions
2642-
.truncate(taken_inst_idx_range.start);
2723+
.truncate(taken_inst_idx_range.start.get());
26432724

2644-
None
2725+
Ok(decoded_format_args)
26452726
};
2646-
remove_format_args_if_possible();
2727+
2728+
match try_decode_and_remove_format_args() {
2729+
Ok(DecodedFormatArgs {}) => {}
2730+
Err(FormatArgsNotRecognized(step)) => {
2731+
if let Some(current_span) = self.current_span {
2732+
let mut warn = self.tcx.sess.struct_span_warn(
2733+
current_span,
2734+
"failed to find and remove `format_args!` construction for this `panic!`",
2735+
);
2736+
2737+
warn.note(
2738+
"compilation may later fail due to leftover `format_args!` internals",
2739+
);
2740+
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`)")
2743+
.help("check `.cargo` and environment variables for potential overrides")
2744+
.help("(or, if not using `spirv-builder` at all, add the flag manually)");
2745+
} else {
2746+
warn.note(format!("[RUST-GPU BUG] bailed from {step}"));
2747+
}
2748+
2749+
warn.emit();
2750+
}
2751+
}
2752+
}
26472753

26482754
// HACK(eddyb) redirect any possible panic call to an abort, to avoid
26492755
// needing to materialize `&core::panic::Location` or `format_args!`.

0 commit comments

Comments
 (0)