diff --git a/crates/rustc_codegen_spirv/src/abi.rs b/crates/rustc_codegen_spirv/src/abi.rs index da589937f4f..d836edf179c 100644 --- a/crates/rustc_codegen_spirv/src/abi.rs +++ b/crates/rustc_codegen_spirv/src/abi.rs @@ -92,15 +92,6 @@ pub(crate) fn provide(providers: &mut Providers) { // 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; @@ -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, diff --git a/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs b/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs index 2c8e4835f3f..16b2a0fd41c 100644 --- a/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs +++ b/crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs @@ -557,16 +557,19 @@ 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 @@ -574,18 +577,40 @@ impl<'tcx> DecodedFormatArgs<'tcx> { { // 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) } 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 diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs b/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs index 601f2b093d5..1db42385494 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs @@ -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; @@ -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: {:#?}", @@ -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"? @@ -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 @@ -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)); + call_args.push(v0); + call_args.push(v1); + assert_eq!(value_len, None); + } _ => unreachable!(), } } diff --git a/tests/compiletests/ui/dis/complex_image_sample_inst.stderr b/tests/compiletests/ui/dis/complex_image_sample_inst.stderr index 0c4548c3709..7ec86b91766 100644 --- a/tests/compiletests/ui/dis/complex_image_sample_inst.stderr +++ b/tests/compiletests/ui/dis/complex_image_sample_inst.stderr @@ -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 diff --git a/tests/compiletests/ui/lang/abi/scalar_pair.rs b/tests/compiletests/ui/lang/abi/scalar_pair.rs new file mode 100644 index 00000000000..ed79952fb66 --- /dev/null +++ b/tests/compiletests/ui/lang/abi/scalar_pair.rs @@ -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) +}