Skip to content

Commit edacf22

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

1 file changed

Lines changed: 142 additions & 23 deletions

File tree

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

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

32483248
// Take `count` instructions, advancing backwards, but returning
32493249
// instructions in their original order (and decoded to `Inst`s).
3250-
let mut try_rev_take = |count| {
3250+
let mut try_rev_take = |count: isize| {
3251+
// HACK(eddyb) this is extremely silly but it's easier to do
3252+
// this than to rely on `Iterator::peekable` or anything else,
3253+
// lower down this file, without messing up the state here.
3254+
let is_peek = count < 0;
3255+
let count = count.unsigned_abs();
3256+
3257+
let mut non_debug_insts_for_peek = is_peek.then(|| non_debug_insts.clone());
3258+
let non_debug_insts = non_debug_insts_for_peek
3259+
.as_mut()
3260+
.unwrap_or(&mut non_debug_insts);
3261+
3262+
// FIXME(eddyb) there might be an easier way to do this,
3263+
// e.g. maybe `map_while` + post-`collect` length check?
32513264
let maybe_rev_insts = (0..count).map(|_| {
32523265
let (i, inst) = non_debug_insts.next_back()?;
3253-
taken_inst_idx_range.start.set(i);
3266+
if !is_peek {
3267+
taken_inst_idx_range.start.set(i);
3268+
}
32543269

32553270
// HACK(eddyb) avoid the logic below that assumes only ID operands
32563271
if inst.class.opcode == Op::CompositeExtract {
@@ -3510,15 +3525,21 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
35103525
}
35113526
}).collect::<Result<_, _>>()?;
35123527

3513-
let rev_ref_arg_ids_with_ty_and_spec = (rev_copies_to_rt_args_array_src_ptrs
3514-
.into_iter())
3515-
.map(|copy_to_rt_args_array_src_ptr| {
3528+
// HACK(eddyb) sometimes there is an extra tuple of refs,
3529+
// nowadays, but MIR opts mean it's not always guaranteed,
3530+
// hopefully it's always uniform across all the arguments.
3531+
let mut maybe_ref_args_tmp_slot_ptr = None;
3532+
3533+
let rev_maybe_ref_arg_ids_with_ty_and_spec = ((0..rt_args_count)
3534+
.rev()
3535+
.zip_eq(rev_copies_to_rt_args_array_src_ptrs))
3536+
.map(|(rt_arg_idx, copy_to_rt_args_array_src_ptr)| {
35163537
let rt_arg_new_call_insts = try_rev_take(4).ok_or_else(|| {
35173538
FormatArgsNotRecognized(
35183539
"fmt::rt::Argument::new call: ran out of instructions".into(),
35193540
)
35203541
})?;
3521-
match rt_arg_new_call_insts[..] {
3542+
let (ref_arg_id, ty, spec) = match rt_arg_new_call_insts[..] {
35223543
[
35233544
Inst::Call(call_ret_id, callee_id, ref call_args),
35243545
Inst::InBoundsAccessChain(tmp_slot_field_ptr, tmp_slot_ptr, 0),
@@ -3542,36 +3563,134 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
35423563
FormatArgsNotRecognized(format!(
35433564
"fmt::rt::Argument::new call sequence ({rt_arg_new_call_insts:?})"
35443565
))
3545-
})
3566+
})?;
3567+
3568+
// HACK(eddyb) `0` (an invalid ID) is later used as a
3569+
// placeholder (see also `maybe_ref_args_tmp_slot_ptr`).
3570+
assert_ne!(ref_arg_id, 0);
3571+
3572+
// HACK(eddyb) `try_rev_take(-2)` is "peeking", not taking.
3573+
let maybe_ref_args_tuple_load_insts = try_rev_take(-2);
3574+
let maybe_ref_arg_id = match maybe_ref_args_tuple_load_insts.as_deref() {
3575+
Some(
3576+
&[
3577+
Inst::InBoundsAccessChain(field_ptr, base_ptr, field_idx),
3578+
Inst::Load(ld_val, ld_src_ptr),
3579+
],
3580+
) if maybe_ref_args_tmp_slot_ptr
3581+
.is_none_or(|expected| base_ptr == expected)
3582+
&& field_idx as usize == rt_arg_idx
3583+
&& (ld_val, ld_src_ptr) == (ref_arg_id, field_ptr) =>
3584+
{
3585+
// HACK(eddyb) consume the peeked instructions.
3586+
try_rev_take(2).unwrap();
3587+
3588+
maybe_ref_args_tmp_slot_ptr = Some(base_ptr);
3589+
3590+
// HACK(eddyb) using `0` (an invalid ID) as a
3591+
// placeholder to require further processing.
3592+
0
3593+
}
3594+
_ => ref_arg_id,
3595+
};
3596+
3597+
Ok((maybe_ref_arg_id, ty, spec))
35463598
})
35473599
.collect::<Result<_, _>>()?;
35483600

35493601
decoded_format_args.ref_arg_ids_with_ty_and_spec =
3550-
rev_ref_arg_ids_with_ty_and_spec;
3602+
rev_maybe_ref_arg_ids_with_ty_and_spec;
35513603
decoded_format_args.ref_arg_ids_with_ty_and_spec.reverse();
3604+
3605+
// HACK(eddyb) see above for context regarding the use of
3606+
// `0` as placeholders and `maybe_ref_args_tmp_slot_ptr`.
3607+
if let Some(ref_args_tmp_slot_ptr) = maybe_ref_args_tmp_slot_ptr {
3608+
for (rt_arg_idx, (maybe_ref_arg_id, ..)) in decoded_format_args
3609+
.ref_arg_ids_with_ty_and_spec
3610+
.iter_mut()
3611+
.enumerate()
3612+
.rev()
3613+
{
3614+
if *maybe_ref_arg_id == 0 {
3615+
let ref_arg_store_insts = try_rev_take(2).ok_or_else(|| {
3616+
FormatArgsNotRecognized(
3617+
"fmt::rt::Argument::new argument store: ran out of instructions".into(),
3618+
)
3619+
})?;
3620+
3621+
*maybe_ref_arg_id = match ref_arg_store_insts[..] {
3622+
[
3623+
Inst::InBoundsAccessChain(field_ptr, base_ptr, field_idx),
3624+
Inst::Store(st_dst_ptr, st_val),
3625+
] if base_ptr == ref_args_tmp_slot_ptr
3626+
&& field_idx as usize == rt_arg_idx
3627+
&& st_dst_ptr == field_ptr =>
3628+
{
3629+
Some(st_val)
3630+
}
3631+
_ => None,
3632+
}
3633+
.ok_or_else(|| {
3634+
FormatArgsNotRecognized(format!(
3635+
"fmt::rt::Argument::new argument store sequence ({ref_arg_store_insts:?})"
3636+
))
3637+
})?;
3638+
}
3639+
}
3640+
}
35523641
}
35533642

35543643
// If the `pieces: &[&str]` slice needs a bitcast, it'll be here.
3555-
let pieces_slice_ptr_id = match try_rev_take(1).as_deref() {
3556-
Some(&[Inst::Bitcast(out_id, in_id)]) if out_id == pieces_slice_ptr_id => in_id,
3644+
// HACK(eddyb) `try_rev_take(-1)` is "peeking", not taking.
3645+
let pieces_slice_ptr_id = match try_rev_take(-1).as_deref() {
3646+
Some(&[Inst::Bitcast(out_id, in_id)]) if out_id == pieces_slice_ptr_id => {
3647+
// HACK(eddyb) consume the peeked instructions.
3648+
try_rev_take(1).unwrap();
3649+
3650+
in_id
3651+
}
35573652
_ => pieces_slice_ptr_id,
35583653
};
35593654
decoded_format_args.const_pieces =
3560-
const_slice_as_elem_ids(pieces_slice_ptr_id, pieces_len).and_then(
3561-
|piece_ids| {
3562-
piece_ids
3563-
.iter()
3564-
.map(|&piece_id| {
3565-
match self.builder.lookup_const_by_id(piece_id)? {
3566-
SpirvConst::Composite(piece) => {
3567-
const_str_as_utf8(piece.try_into().ok()?)
3568-
}
3569-
_ => None,
3655+
match const_slice_as_elem_ids(pieces_slice_ptr_id, pieces_len) {
3656+
Some(piece_ids) => piece_ids
3657+
.iter()
3658+
.map(
3659+
|&piece_id| match self.builder.lookup_const_by_id(piece_id)? {
3660+
SpirvConst::Composite(piece) => {
3661+
const_str_as_utf8(piece.try_into().ok()?)
35703662
}
3571-
})
3572-
.collect::<Option<_>>()
3663+
_ => None,
3664+
},
3665+
)
3666+
.collect::<Option<_>>(),
3667+
// HACK(eddyb) minor upstream blunder results in at
3668+
// least one instance of a runtime `[&str; 1]` array,
3669+
// see also this comment left on the responsible PR:
3670+
// https://github.com/rust-lang/rust/pull/129658#discussion_r2181834781
3671+
// HACK(eddyb) `try_rev_take(-4)` is "peeking", not taking.
3672+
None if pieces_len == 1 => match try_rev_take(-4).as_deref() {
3673+
Some(
3674+
&[
3675+
Inst::InBoundsAccessChain2(field0_ptr, array_ptr_0, 0, 0),
3676+
Inst::Store(st0_dst_ptr, st0_val),
3677+
Inst::InBoundsAccessChain2(field1_ptr, array_ptr_1, 0, 1),
3678+
Inst::Store(st1_dst_ptr, st1_val),
3679+
],
3680+
) if [array_ptr_0, array_ptr_1] == [pieces_slice_ptr_id; 2]
3681+
&& st0_dst_ptr == field0_ptr
3682+
&& st1_dst_ptr == field1_ptr =>
3683+
{
3684+
// HACK(eddyb) consume the peeked instructions.
3685+
try_rev_take(4).unwrap();
3686+
3687+
const_str_as_utf8(&[st0_val, st1_val])
3688+
.map(|s| [s].into_iter().collect())
3689+
}
3690+
_ => None,
35733691
},
3574-
);
3692+
None => None,
3693+
};
35753694

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

0 commit comments

Comments
 (0)