Skip to content

Commit c980c00

Browse files
eddybeddyb
committed
builder: support suboptimal format_args! codegen (extra copies etc.).
1 parent 50ff07b commit c980c00

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
@@ -3284,10 +3284,25 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
32843284

32853285
// Take `count` instructions, advancing backwards, but returning
32863286
// instructions in their original order (and decoded to `Inst`s).
3287-
let mut try_rev_take = |count| {
3287+
let mut try_rev_take = |count: isize| {
3288+
// HACK(eddyb) this is extremely silly but it's easier to do
3289+
// this than to rely on `Iterator::peekable` or anything else,
3290+
// lower down this file, without messing up the state here.
3291+
let is_peek = count < 0;
3292+
let count = count.unsigned_abs();
3293+
3294+
let mut non_debug_insts_for_peek = is_peek.then(|| non_debug_insts.clone());
3295+
let non_debug_insts = non_debug_insts_for_peek
3296+
.as_mut()
3297+
.unwrap_or(&mut non_debug_insts);
3298+
3299+
// FIXME(eddyb) there might be an easier way to do this,
3300+
// e.g. maybe `map_while` + post-`collect` length check?
32883301
let maybe_rev_insts = (0..count).map(|_| {
32893302
let (i, inst) = non_debug_insts.next_back()?;
3290-
taken_inst_idx_range.start.set(i);
3303+
if !is_peek {
3304+
taken_inst_idx_range.start.set(i);
3305+
}
32913306

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

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

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

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

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

0 commit comments

Comments
 (0)