scalar pair: remove abi patching scalar pairs to scalars, fixup fmt args and entry point

Author: LegNeato

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;
@@ -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,

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,7 +12,9 @@ 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, LayoutTypeCodegenMethods, ConstCodegenMethods, MiscCodegenMethods as _,
+};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::MultiSpan;
 use rustc_hir as hir;
@@ -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: {:#?}",
@@ -517,14 +504,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 +626,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 +672,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!(),
             }
         }

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