Skip to content

Commit 708ef68

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

File tree

1 file changed

+142
-23
lines changed

1 file changed

+142
-23
lines changed

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

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

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

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

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

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

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

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

0 commit comments

Comments
 (0)