Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions crates/rustc_codegen_spirv/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,6 @@ pub(crate) fn provide(providers: &mut Providers) {
// <https://github.com/rust-lang/rust/commit/eaaa03faf77b157907894a4207d8378ecaec7b45>
arg.make_direct_deprecated();

// FIXME(eddyb) detect `#[rust_gpu::vector::v1]` more specifically,
// to avoid affecting anything should actually be passed as a pair.
if let PassMode::Pair(..) = arg.mode {
// HACK(eddyb) this avoids breaking e.g. `&[T]` pairs.
if let TyKind::Adt(..) = arg.layout.ty.kind() {
arg.mode = PassMode::Direct(ArgAttributes::new());
}
}

// Avoid pointlessly passing ZSTs, just like the official Rust ABI.
if arg.layout.is_zst() {
arg.mode = PassMode::Ignore;
Expand Down Expand Up @@ -461,7 +452,7 @@ pub fn scalar_pair_element_backend_type<'tcx>(
ty: TyAndLayout<'tcx>,
index: usize,
) -> Word {
let [a, b] = match ty.layout.backend_repr() {
let [a, b] = match ty.backend_repr {
BackendRepr::ScalarPair(a, b) => [a, b],
other => span_bug!(
span,
Expand Down
49 changes: 37 additions & 12 deletions crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,35 +557,60 @@ impl<'tcx> DecodedFormatArgs<'tcx> {
if let Some((template_id, template_ty_id, rt_args_ptr_id, rt_args_ptr_ty_id)) =
split_fmt_args
{
let ctor = if let (Some(template_len), Some(rt_args_count)) = (
if let (Some(template_len), Some(rt_args_count)) = (
const_ptr_to_composite_len(template_id)
.or_else(|| array_len_from_ptr_type(template_ty_id)),
const_ptr_to_composite_len(rt_args_ptr_id)
.or_else(|| array_len_from_ptr_type(rt_args_ptr_ty_id)),
) {
FmtArgsCtor::NewTemplate {
template_len,
rt_args_count,
}
(
FmtArgsCtor::NewTemplate {
template_len,
rt_args_count,
},
SmallVec::<[Word; 8]>::from_slice(&[template_id, rt_args_ptr_id]),
)
} else if let Some(&[Inst::Call(_, callee_id, ref call_args)]) =
try_rev_take(-1).as_deref()
&& call_args.len() == 2
&& [call_args[0], call_args[1]] == [template_id, rt_args_ptr_id]
{
// Consume the matched call instruction.
try_rev_take(1).unwrap();
lookup_fmt_args_ctor(callee_id)?
(
lookup_fmt_args_ctor(callee_id)?,
SmallVec::<[Word; 8]>::from_slice(&[template_id, rt_args_ptr_id]),
)
} else if let Some(
&[
Inst::Call(call_ret_id, callee_id, ref call_args),
Inst::CompositeExtract(extracted0, from0, 0),
Inst::CompositeExtract(extracted1, from1, 1),
],
) = try_rev_take(-3).as_deref()
&& [from0, from1] == [call_ret_id; 2]
&& [extracted0, extracted1] == [template_id, rt_args_ptr_id]
{
// Newer rustc, since `BackendRepr::ScalarPair` args are no
// longer forced to `PassMode::Direct`, returns the whole
// `fmt::Arguments` from its `new_*` constructor as a scalar
// pair, and splits it (via `OpCompositeExtract`s) into the
// two scalar values passed to the panic entry-point.
//
// The constructor's own arguments (i.e. `pieces`/`template`
// and the `rt::Argument` slice pointers) still carry the
// recoverable const data, so use those, like the aggregate
// (non-split) `Call`+`extract`+`insert` case does below.
let call_args_storage = call_args.iter().copied().collect();
// Consume the matched call + both `OpCompositeExtract`s.
try_rev_take(3).unwrap();
(lookup_fmt_args_ctor(callee_id)?, call_args_storage)
Comment on lines +584 to +607

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason I don't like this has to do with it feeling misplaced, likely a consequence of the split_fmt_args changes from months ago, which I might eventually revisit (and shouldn't block this PR).

} else {
// We failed to recover constructor metadata for an already-split
// `fmt::Arguments` value. Keep panic lowering sound by falling
// back to an unknown panic message, without requiring decompilation.
return Ok(decoded_format_args);
};

(
ctor,
SmallVec::<[Word; 8]>::from_slice(&[template_id, rt_args_ptr_id]),
)
}
} else {
// Newer rustc can pass the `fmt::Arguments::new_*` result directly to
// panic entry points (single trailing call), while older versions go
Expand Down
62 changes: 35 additions & 27 deletions crates/rustc_codegen_spirv/src/codegen_cx/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ use rspirv::spirv::{
BuiltIn, Decoration, Dim, ExecutionModel, FunctionControl, StorageClass, Word,
};
use rustc_abi::FieldsShape;
use rustc_codegen_ssa::traits::{BaseTypeCodegenMethods, BuilderMethods, MiscCodegenMethods as _};
use rustc_codegen_ssa::traits::{
BaseTypeCodegenMethods, BuilderMethods, ConstCodegenMethods, LayoutTypeCodegenMethods,
MiscCodegenMethods as _,
};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::MultiSpan;
use rustc_hir as hir;
Expand Down Expand Up @@ -87,22 +90,7 @@ impl<'tcx> CodegenCx<'tcx> {
};
for (arg_abi, hir_param) in fn_abi.args.iter().zip(hir_params) {
match arg_abi.mode {
PassMode::Direct(_) | PassMode::Ignore => {}
PassMode::Pair(..) => {
// FIXME(eddyb) implement `ScalarPair` `Input`s, or change
// the `FnAbi` readjustment to only use `PassMode::Pair` for
// pointers to `!Sized` types, but not other `ScalarPair`s.
if !matches!(arg_abi.layout.ty.kind(), ty::Ref(..)) {
self.tcx.dcx().span_err(
hir_param.ty_span,
format!(
"entry point parameter type not yet supported \
(`{}` has `ScalarPair` ABI but is not a `&T`)",
arg_abi.layout.ty
),
);
}
}
PassMode::Direct(_) | PassMode::Pair(..) | PassMode::Ignore => {}
_ => span_bug!(
hir_param.ty_span,
"query hooks should've made this `PassMode` impossible: {:#?}",
Expand Down Expand Up @@ -517,14 +505,6 @@ impl<'tcx> CodegenCx<'tcx> {
vs layout:\n{value_layout:#?}",
entry_arg_abi.layout.ty
);
if is_pair && !is_unsized {
// If PassMode is Pair, then we need to fill in the second part of the pair with a
// value. We currently only do that with unsized types, so if a type is a pair for some
// other reason (e.g. a tuple), we bail.
self.tcx
.dcx()
.span_fatal(hir_param.ty_span, "pair type not supported yet")
}
// FIXME(eddyb) should this talk about "typed buffers" instead of "interface blocks"?
// FIXME(eddyb) should we talk about "descriptor indexing" or
// actually use more reasonable terms like "resource arrays"?
Expand Down Expand Up @@ -647,8 +627,8 @@ impl<'tcx> CodegenCx<'tcx> {
}
}

let value_len = if is_pair {
// We've already emitted an error, fill in a placeholder value
let value_len = if is_pair && is_unsized {
// For wide references (e.g., slices), the second component is a length.
Some(bx.undef(self.type_isize()))
} else {
None
Expand Down Expand Up @@ -693,6 +673,34 @@ impl<'tcx> CodegenCx<'tcx> {
call_args.push(value);
assert_eq!(value_len, None);
}
PassMode::Pair(..) => {
// Load both elements of the scalar pair from the input variable.
assert_eq!(storage_class, Ok(StorageClass::Input));
let layout = entry_arg_abi.layout;
let (a, b) = match layout.backend_repr {
rustc_abi::BackendRepr::ScalarPair(a, b) => (a, b),
other => span_bug!(
hir_param.ty_span,
"ScalarPair expected for entry param, found {other:?}"
),
};
let b_offset = a
.primitive()
.size(self)
.align_to(b.primitive().align(self).abi);

let elem0_ty = self.scalar_pair_element_backend_type(layout, 0, false);
let elem1_ty = self.scalar_pair_element_backend_type(layout, 1, false);

let base_ptr = value_ptr.unwrap();
let ptr1 = bx.inbounds_ptradd(base_ptr, self.const_usize(b_offset.bytes()));

let v0 = bx.load(elem0_ty, base_ptr, layout.align.abi);
let v1 = bx.load(elem1_ty, ptr1, layout.align.restrict_for_offset(b_offset));
Comment on lines +687 to +699

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be possible to replace this with a call to load_operand.

call_args.push(v0);
call_args.push(v1);
assert_eq!(value_len, None);
}
_ => unreachable!(),
}
}
Expand Down
26 changes: 12 additions & 14 deletions tests/compiletests/ui/dis/complex_image_sample_inst.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,17 @@
%4 = OpFunctionParameter %2
%5 = OpFunctionParameter %6
%7 = OpFunctionParameter %6
%8 = OpLabel
%9 = OpCompositeExtract %10 %5 0
%11 = OpCompositeExtract %10 %5 1
%12 = OpCompositeConstruct %6 %9 %11
%13 = OpCompositeExtract %10 %7 0
%14 = OpCompositeExtract %10 %7 1
%15 = OpCompositeConstruct %6 %13 %14
OpLine %16 29 13
%17 = OpAccessChain %18 %19 %20
OpLine %16 30 13
%21 = OpLoad %22 %17
OpLine %16 34 13
%23 = OpImageSampleProjExplicitLod %2 %21 %4 Grad %12 %15
%8 = OpFunctionParameter %6
%9 = OpFunctionParameter %6
%10 = OpLabel
%11 = OpCompositeConstruct %12 %5 %7
%13 = OpCompositeConstruct %12 %8 %9
OpLine %14 29 13
%15 = OpAccessChain %16 %17 %18
OpLine %14 30 13
%19 = OpLoad %20 %15
OpLine %14 34 13
%21 = OpImageSampleProjExplicitLod %2 %19 %4 Grad %11 %13
OpNoLine
OpReturnValue %23
OpReturnValue %21
OpFunctionEnd
27 changes: 27 additions & 0 deletions tests/compiletests/ui/lang/abi/scalar_pair.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// build-pass
// compile-flags: -C target-feature=+Int64

use spirv_std::spirv;

#[spirv(fragment)]
pub fn main_future_proof(
#[spirv(flat)] input: (u64, u32),
out: &mut (u64, u32),
#[spirv(storage_buffer, descriptor_set = 0, binding = 0)] buffer_in: &(u64, u32),
#[spirv(storage_buffer, descriptor_set = 1, binding = 0)] buffer_out: &mut (u64, u32),
) {
*out = trans0(trans_ref(buffer_in));
*buffer_out = trans1(input);
}

pub fn trans0(arg: (u64, u32)) -> (u64, u32) {
(arg.0 + 1, arg.1 - 1)
}

pub fn trans1((a, b): (u64, u32)) -> (u64, u32) {
(a * 2, b * 3)
}

pub fn trans_ref((a, b): &(u64, u32)) -> (u64, u32) {
(a - 1, b - 1)
}
Loading