Skip to content

Commit e16e124

Browse files
eddybeddyb
committed
builder: support suboptimal format_args! codegen (extra copies etc.).
1 parent 9331e20 commit e16e124

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 {
@@ -3547,15 +3562,21 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
35473562
}
35483563
}).collect::<Result<_, _>>()?;
35493564

3550-
let rev_ref_arg_ids_with_ty_and_spec = (rev_copies_to_rt_args_array_src_ptrs
3551-
.into_iter())
3552-
.map(|copy_to_rt_args_array_src_ptr| {
3565+
// HACK(eddyb) sometimes there is an extra tuple of refs,
3566+
// nowadays, but MIR opts mean it's not always guaranteed,
3567+
// hopefully it's always uniform across all the arguments.
3568+
let mut maybe_ref_args_tmp_slot_ptr = None;
3569+
3570+
let rev_maybe_ref_arg_ids_with_ty_and_spec = ((0..rt_args_count)
3571+
.rev()
3572+
.zip_eq(rev_copies_to_rt_args_array_src_ptrs))
3573+
.map(|(rt_arg_idx, copy_to_rt_args_array_src_ptr)| {
35533574
let rt_arg_new_call_insts = try_rev_take(4).ok_or_else(|| {
35543575
FormatArgsNotRecognized(
35553576
"fmt::rt::Argument::new call: ran out of instructions".into(),
35563577
)
35573578
})?;
3558-
match rt_arg_new_call_insts[..] {
3579+
let (ref_arg_id, ty, spec) = match rt_arg_new_call_insts[..] {
35593580
[
35603581
Inst::Call(call_ret_id, callee_id, ref call_args),
35613582
Inst::InBoundsAccessChain(tmp_slot_field_ptr, tmp_slot_ptr, 0),
@@ -3579,36 +3600,136 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
35793600
FormatArgsNotRecognized(format!(
35803601
"fmt::rt::Argument::new call sequence ({rt_arg_new_call_insts:?})"
35813602
))
3582-
})
3603+
})?;
3604+
3605+
// HACK(eddyb) `0` (an invalid ID) is later used as a
3606+
// placeholder (see also `maybe_ref_args_tmp_slot_ptr`).
3607+
assert_ne!(ref_arg_id, 0);
3608+
3609+
// HACK(eddyb) `try_rev_take(-2)` is "peeking", not taking.
3610+
let maybe_ref_args_tuple_load_insts = try_rev_take(-2);
3611+
let maybe_ref_arg_id = match maybe_ref_args_tuple_load_insts.as_deref() {
3612+
Some(
3613+
&[
3614+
Inst::InBoundsAccessChain(field_ptr, base_ptr, field_idx),
3615+
Inst::Load(ld_val, ld_src_ptr),
3616+
],
3617+
) if maybe_ref_args_tmp_slot_ptr
3618+
.is_none_or(|expected| base_ptr == expected)
3619+
&& field_idx as usize == rt_arg_idx
3620+
&& (ld_val, ld_src_ptr) == (ref_arg_id, field_ptr) =>
3621+
{
3622+
// HACK(eddyb) consume the peeked instructions.
3623+
try_rev_take(2).unwrap();
3624+
3625+
maybe_ref_args_tmp_slot_ptr = Some(base_ptr);
3626+
3627+
// HACK(eddyb) using `0` (an invalid ID) as a
3628+
// placeholder to require further processing.
3629+
0
3630+
}
3631+
_ => ref_arg_id,
3632+
};
3633+
3634+
Ok((maybe_ref_arg_id, ty, spec))
35833635
})
35843636
.collect::<Result<_, _>>()?;
35853637

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

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

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

0 commit comments

Comments
 (0)