support scalar pairs properly

crates/rustc_codegen_spirv/src/abi.rs

@@ -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;
@@ -490,7 +481,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,

crates/rustc_codegen_spirv/src/builder/format_args_decompiler.rs

@@ -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)
                 } 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

crates/rustc_codegen_spirv/src/codegen_cx/entry.rs

@@ -12,6 +12,8 @@ use rspirv::spirv::{
     BuiltIn, Decoration, Dim, ExecutionModel, FunctionControl, StorageClass, Word,
 };
 use rustc_abi::FieldsShape;
+use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
+use rustc_codegen_ssa::mir::place::PlaceRef;
 use rustc_codegen_ssa::traits::{BaseTypeCodegenMethods, BuilderMethods, MiscCodegenMethods as _};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::MultiSpan;
@@ -87,22 +89,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: {:#?}",
@@ -504,27 +491,19 @@ impl<'tcx> CodegenCx<'tcx> {
         // Certain storage classes require an `OpTypeStruct` decorated with `Block`,
         // which we represent with `SpirvType::InterfaceBlock` (see its doc comment).
         // This "interface block" construct is also required for "runtime arrays".
-        let is_unsized = self.lookup_type(value_spirv_type).sizeof(self).is_none();
+        let pointee_is_unsized = self.lookup_type(value_spirv_type).sizeof(self).is_none();
         let is_pair = matches!(entry_arg_abi.mode, PassMode::Pair(..));
-        let is_unsized_with_len = is_pair && is_unsized;
+        let is_unsized_with_len = is_pair && pointee_is_unsized;
         // HACK(eddyb) sanity check because we get the same information in two
         // very different ways, and going out of sync could cause subtle issues.
         assert_eq!(
             is_unsized_with_len,
             value_layout.is_unsized(),
             "`{}` param mismatch in call ABI (is_pair={is_pair}) + \
-             SPIR-V type (is_unsized={is_unsized}) \
+             SPIR-V type (is_unsized={pointee_is_unsized}) \
              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"?
@@ -591,7 +570,7 @@ impl<'tcx> CodegenCx<'tcx> {
 
                 Some(len.with_type(len_spirv_type))
             } else {
-                if is_unsized {
+                if pointee_is_unsized {
                     // It's OK to use a RuntimeArray<u32> and not have a length parameter, but
                     // it's just nicer ergonomics to use a slice.
                     self.tcx
@@ -621,7 +600,7 @@ impl<'tcx> CodegenCx<'tcx> {
                         }
                     }
                     _ => {
-                        if is_unsized {
+                        if pointee_is_unsized {
                             self.tcx.dcx().span_err(
                                 hir_param.ty_span,
                                 "only RuntimeArray is supported, not other unsized types",
@@ -633,7 +612,7 @@ impl<'tcx> CodegenCx<'tcx> {
                 // FIXME(eddyb) determine, based on the type, what kind of type
                 // this is, to narrow it further to e.g. "buffer in a non-buffer
                 // storage class" or "storage class expects fixed data sizes".
-                if is_unsized {
+                if pointee_is_unsized {
                     self.tcx.dcx().span_fatal(
                         hir_param.ty_span,
                         format!(
@@ -647,7 +626,8 @@ impl<'tcx> CodegenCx<'tcx> {
                 }
             }
 
-            let value_len = if is_pair {
+            let value_len = if is_pair && pointee_is_unsized {
+                // A slice *cannot* be passed as anything other than a StorageBuffer or Uniform
                 // We've already emitted an error, fill in a placeholder value
                 Some(bx.undef(self.type_isize()))
             } else {
@@ -693,6 +673,22 @@ 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 OperandRef {
+                        val: OperandValue::Pair(v0, v1),
+                        ..
+                    } = bx.load_operand(PlaceRef::new_sized(
+                        value_ptr.unwrap(),
+                        entry_arg_abi.layout,
+                    ))
+                    else {
+                        unreachable!();
+                    };
+                    call_args.extend([v0, v1]);
+                    assert_eq!(value_len, None);
+                }
                 _ => unreachable!(),
             }
         }

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

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)
+}