Skip to content

Commit 431fe62

Browse files
eddybeddyb
committed
builder: support suboptimal format_args! codegen (extra copies etc.).
1 parent ea8e50c commit 431fe62

File tree

1 file changed

+144
-23
lines changed

1 file changed

+144
-23
lines changed

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

Lines changed: 144 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3398,10 +3398,25 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
33983398

33993399
// Take `count` instructions, advancing backwards, but returning
34003400
// instructions in their original order (and decoded to `Inst`s).
3401-
let mut try_rev_take = |count| {
3401+
let mut try_rev_take = |count: isize| {
3402+
// HACK(eddyb) this is extremely silly but it's easier to do
3403+
// this than to rely on `Iterator::peekable` or anything else,
3404+
// lower down this file, without messing up the state here.
3405+
let is_peek = count < 0;
3406+
let count = count.unsigned_abs();
3407+
3408+
let mut non_debug_insts_for_peek = is_peek.then(|| non_debug_insts.clone());
3409+
let non_debug_insts = non_debug_insts_for_peek
3410+
.as_mut()
3411+
.unwrap_or(&mut non_debug_insts);
3412+
3413+
// FIXME(eddyb) there might be an easier way to do this,
3414+
// e.g. maybe `map_while` + post-`collect` length check?
34023415
let maybe_rev_insts = (0..count).map(|_| {
34033416
let (i, inst) = non_debug_insts.next_back()?;
3404-
taken_inst_idx_range.start.set(i);
3417+
if !is_peek {
3418+
taken_inst_idx_range.start.set(i);
3419+
}
34053420

34063421
// HACK(eddyb) avoid the logic below that assumes only ID operands
34073422
if inst.class.opcode == Op::CompositeExtract {
@@ -3552,15 +3567,21 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
35523567
}
35533568
}).collect::<Result<_, _>>()?;
35543569

3555-
let rev_ref_arg_ids_with_ty_and_spec = (rev_copies_to_rt_args_array_src_ptrs
3556-
.into_iter())
3557-
.map(|copy_to_rt_args_array_src_ptr| {
3570+
// HACK(eddyb) sometimes there is an extra tuple of refs,
3571+
// nowadays, but MIR opts mean it's not always guaranteed,
3572+
// hopefully it's always uniform across all the arguments.
3573+
let mut maybe_ref_args_tmp_slot_ptr = None;
3574+
3575+
let rev_maybe_ref_arg_ids_with_ty_and_spec = ((0..rt_args_count)
3576+
.rev()
3577+
.zip_eq(rev_copies_to_rt_args_array_src_ptrs))
3578+
.map(|(rt_arg_idx, copy_to_rt_args_array_src_ptr)| {
35583579
let rt_arg_new_call_insts = try_rev_take(4).ok_or_else(|| {
35593580
FormatArgsNotRecognized(
35603581
"fmt::rt::Argument::new call: ran out of instructions".into(),
35613582
)
35623583
})?;
3563-
match rt_arg_new_call_insts[..] {
3584+
let (ref_arg_id, ty, spec) = match rt_arg_new_call_insts[..] {
35643585
[
35653586
Inst::Call(call_ret_id, callee_id, ref call_args),
35663587
Inst::InBoundsAccessChain(tmp_slot_field_ptr, tmp_slot_ptr, 0),
@@ -3584,36 +3605,136 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
35843605
FormatArgsNotRecognized(format!(
35853606
"fmt::rt::Argument::new call sequence ({rt_arg_new_call_insts:?})"
35863607
))
3587-
})
3608+
})?;
3609+
3610+
// HACK(eddyb) `0` (an invalid ID) is later used as a
3611+
// placeholder (see also `maybe_ref_args_tmp_slot_ptr`).
3612+
assert_ne!(ref_arg_id, 0);
3613+
3614+
// HACK(eddyb) `try_rev_take(-2)` is "peeking", not taking.
3615+
let maybe_ref_args_tuple_load_insts = try_rev_take(-2);
3616+
let maybe_ref_arg_id = match maybe_ref_args_tuple_load_insts.as_deref() {
3617+
Some(
3618+
&[
3619+
Inst::InBoundsAccessChain(field_ptr, base_ptr, field_idx),
3620+
Inst::Load(ld_val, ld_src_ptr),
3621+
],
3622+
) if maybe_ref_args_tmp_slot_ptr
3623+
.is_none_or(|expected| base_ptr == expected)
3624+
&& field_idx as usize == rt_arg_idx
3625+
&& (ld_val, ld_src_ptr) == (ref_arg_id, field_ptr) =>
3626+
{
3627+
// HACK(eddyb) consume the peeked instructions.
3628+
try_rev_take(2).unwrap();
3629+
3630+
maybe_ref_args_tmp_slot_ptr = Some(base_ptr);
3631+
3632+
// HACK(eddyb) using `0` (an invalid ID) as a
3633+
// placeholder to require further processing.
3634+
0
3635+
}
3636+
_ => ref_arg_id,
3637+
};
3638+
3639+
Ok((maybe_ref_arg_id, ty, spec))
35883640
})
35893641
.collect::<Result<_, _>>()?;
35903642

35913643
decoded_format_args.ref_arg_ids_with_ty_and_spec =
3592-
rev_ref_arg_ids_with_ty_and_spec;
3644+
rev_maybe_ref_arg_ids_with_ty_and_spec;
35933645
decoded_format_args.ref_arg_ids_with_ty_and_spec.reverse();
3646+
3647+
// HACK(eddyb) see above for context regarding the use of
3648+
// `0` as placeholders and `maybe_ref_args_tmp_slot_ptr`.
3649+
if let Some(ref_args_tmp_slot_ptr) = maybe_ref_args_tmp_slot_ptr {
3650+
for (rt_arg_idx, (maybe_ref_arg_id, ..)) in decoded_format_args
3651+
.ref_arg_ids_with_ty_and_spec
3652+
.iter_mut()
3653+
.enumerate()
3654+
.rev()
3655+
{
3656+
if *maybe_ref_arg_id == 0 {
3657+
let ref_arg_store_insts = try_rev_take(2).ok_or_else(|| {
3658+
FormatArgsNotRecognized(
3659+
"fmt::rt::Argument::new argument store: ran out of instructions".into(),
3660+
)
3661+
})?;
3662+
3663+
*maybe_ref_arg_id = match ref_arg_store_insts[..] {
3664+
[
3665+
Inst::InBoundsAccessChain(field_ptr, base_ptr, field_idx),
3666+
Inst::Store(st_dst_ptr, st_val),
3667+
] if base_ptr == ref_args_tmp_slot_ptr
3668+
&& field_idx as usize == rt_arg_idx
3669+
&& st_dst_ptr == field_ptr =>
3670+
{
3671+
Some(st_val)
3672+
}
3673+
_ => None,
3674+
}
3675+
.ok_or_else(|| {
3676+
FormatArgsNotRecognized(format!(
3677+
"fmt::rt::Argument::new argument store sequence ({ref_arg_store_insts:?})"
3678+
))
3679+
})?;
3680+
}
3681+
}
3682+
}
35943683
}
35953684

35963685
// If the `pieces: &[&str]` slice needs a bitcast, it'll be here.
3597-
let pieces_slice_ptr_id = match try_rev_take(1).as_deref() {
3598-
Some(&[Inst::Bitcast(out_id, in_id)]) if out_id == pieces_slice_ptr_id => in_id,
3686+
// HACK(eddyb) `try_rev_take(-1)` is "peeking", not taking.
3687+
let pieces_slice_ptr_id = match try_rev_take(-1).as_deref() {
3688+
Some(&[Inst::Bitcast(out_id, in_id)]) if out_id == pieces_slice_ptr_id => {
3689+
// HACK(eddyb) consume the peeked instructions.
3690+
try_rev_take(1).unwrap();
3691+
3692+
in_id
3693+
}
35993694
_ => pieces_slice_ptr_id,
36003695
};
36013696
decoded_format_args.const_pieces =
3602-
const_slice_as_elem_ids(pieces_slice_ptr_id, pieces_len).and_then(
3603-
|piece_ids| {
3604-
piece_ids
3605-
.iter()
3606-
.map(|&piece_id| {
3607-
match self.builder.lookup_const_by_id(piece_id)? {
3608-
SpirvConst::Composite(piece) => {
3609-
const_str_as_utf8(piece.try_into().ok()?)
3610-
}
3611-
_ => None,
3697+
match const_slice_as_elem_ids(pieces_slice_ptr_id, pieces_len) {
3698+
Some(piece_ids) => piece_ids
3699+
.iter()
3700+
.map(
3701+
|&piece_id| match self.builder.lookup_const_by_id(piece_id)? {
3702+
SpirvConst::Composite(piece) => {
3703+
const_str_as_utf8(piece.try_into().ok()?)
36123704
}
3613-
})
3614-
.collect::<Option<_>>()
3705+
_ => None,
3706+
},
3707+
)
3708+
.collect::<Option<_>>(),
3709+
// HACK(eddyb) minor upstream blunder results in at
3710+
// least one instance of a runtime `[&str; 1]` array,
3711+
// see also this comment left on the responsible PR:
3712+
// https://github.com/rust-lang/rust/pull/129658#discussion_r2181834781
3713+
// HACK(eddyb) `try_rev_take(-5)` is "peeking", not taking.
3714+
None if pieces_len == 1 => match try_rev_take(-5).as_deref() {
3715+
Some(
3716+
&[
3717+
Inst::InBoundsAccessChain(elem0_ptr, array_ptr, 0),
3718+
Inst::InBoundsAccessChain(field0_ptr, base_ptr_0, 0),
3719+
Inst::Store(st0_dst_ptr, st0_val),
3720+
Inst::InBoundsAccessChain(field1_ptr, base_ptr_1, 1),
3721+
Inst::Store(st1_dst_ptr, st1_val),
3722+
],
3723+
) if array_ptr == pieces_slice_ptr_id
3724+
&& [base_ptr_0, base_ptr_1] == [elem0_ptr; 2]
3725+
&& st0_dst_ptr == field0_ptr
3726+
&& st1_dst_ptr == field1_ptr =>
3727+
{
3728+
// HACK(eddyb) consume the peeked instructions.
3729+
try_rev_take(5).unwrap();
3730+
3731+
const_str_as_utf8(&[st0_val, st1_val])
3732+
.map(|s| [s].into_iter().collect())
3733+
}
3734+
_ => None,
36153735
},
3616-
);
3736+
None => None,
3737+
};
36173738

36183739
// Keep all instructions up to (but not including) the last one
36193740
// confirmed above to be the first instruction of `format_args!`.

0 commit comments

Comments
 (0)