Skip to content

Commit 31e3fdb

Browse files
committed
abi: explicitly unpack ScalarPair newtypes.
1 parent 0c65cf3 commit 31e3fdb

File tree

2 files changed

+76
-0
lines changed

2 files changed

+76
-0
lines changed

crates/rustc_codegen_spirv/src/abi.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,40 @@ impl<'tcx> ConvSpirvType<'tcx> for TyAndLayout<'tcx> {
370370
.def_with_name(cx, span, TyLayoutNameKey::from(*self)),
371371
Abi::Scalar(ref scalar) => trans_scalar(cx, span, *self, scalar, Size::ZERO),
372372
Abi::ScalarPair(ref a, ref b) => {
373+
// NOTE(eddyb) unlike `Abi::Scalar`'s simpler newtype-unpacking
374+
// behavior, `Abi::ScalarPair` can be composed in two ways:
375+
// * two `Abi::Scalar` fields (and any number of ZST fields),
376+
// gets handled the same as a `struct { a, b }`, further below
377+
// * an `Abi::ScalarPair` field (and any number of ZST fields),
378+
// which requires more work to allow taking a reference to
379+
// that field, and there are two potential approaches:
380+
// 1. wrapping that field's SPIR-V type in a single-field
381+
// `OpTypeStruct` - this has the disadvantage that GEPs
382+
// would have to inject an extra `0` field index, and other
383+
// field-related operations would also need additional work
384+
// 2. reusing that field's SPIR-V type, instead of defining
385+
// a new one, offering the `(a, b)` shape `rustc_codegen_ssa`
386+
// expects, while letting noop pointercasts access the sole
387+
// `Abi::ScalarPair` field - this is the approach taken here
388+
let mut non_zst_fields = (0..self.fields.count())
389+
.map(|i| (i, self.field(cx, i)))
390+
.filter(|(_, field)| !field.is_zst());
391+
let sole_non_zst_field = match (non_zst_fields.next(), non_zst_fields.next()) {
392+
(Some(field), None) => Some(field),
393+
_ => None,
394+
};
395+
if let Some((i, field)) = sole_non_zst_field {
396+
// Only unpack a newtype if the field and the newtype line up
397+
// perfectly, in every way that could potentially affect ABI.
398+
if self.fields.offset(i) == Size::ZERO
399+
&& field.size == self.size
400+
&& field.align == self.align
401+
&& field.abi == self.abi
402+
{
403+
return field.spirv_type(span, cx);
404+
}
405+
}
406+
373407
// Note: We can't use auto_struct_layout here because the spirv types here might be undefined due to
374408
// recursive pointer types.
375409
let a_offset = Size::ZERO;

tests/ui/lang/issue-836.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Test that newtypes of `ScalarPair` can have references taken to their field.
2+
3+
// build-pass
4+
5+
use spirv_std as _;
6+
7+
struct Newtype<T>(T);
8+
9+
impl<T> Newtype<T> {
10+
fn get(&self) -> &T {
11+
&self.0
12+
}
13+
}
14+
15+
impl Newtype<&[u32]> {
16+
fn slice_get(&self) -> &&[u32] {
17+
&self.0
18+
}
19+
}
20+
21+
impl<T: core::ops::Deref<Target = [u32]>> Newtype<T> {
22+
fn deref_index(&self, i: usize) -> &u32 {
23+
&self.0[i]
24+
}
25+
}
26+
27+
struct CustomPair(u32, u32);
28+
29+
#[spirv(fragment)]
30+
pub fn main(
31+
#[spirv(descriptor_set = 0, binding = 0, storage_buffer)] slice: &[u32],
32+
#[spirv(flat)] out: &mut u32,
33+
) {
34+
let newtype_slice = Newtype(slice);
35+
*out = newtype_slice.get()[0];
36+
*out += newtype_slice.slice_get()[1];
37+
*out += newtype_slice.deref_index(2);
38+
39+
let newtype_custom_pair = Newtype(CustomPair(*out, *out + 1));
40+
*out += newtype_custom_pair.get().0;
41+
*out += newtype_custom_pair.get().1;
42+
}

0 commit comments

Comments
 (0)