Skip to content

Commit 67f1ff2

Browse files
LegNeatoFirestar99
authored andcommitted
Fix codegen of Memory-repr enum payload accesses
Fixes #615. After the nightly-2026-05-22 upgrade, `TryFromIntError` is no longer a ZST, so `Result<u32, TryFromIntError>` is laid out as `BackendRepr::Memory` instead of `ScalarPair`. Multi-variant enums with this layout were translated to a tag-only `OpTypeStruct`, so payload field accesses couldn't be recovered into a valid `OpAccessChain`. The SPIR-V type now also includes each variant's non-ZST payload fields at their absolute offsets, so payload accesses work.
1 parent fa9fa14 commit 67f1ff2

4 files changed

Lines changed: 219 additions & 0 deletions

File tree

crates/rustc_codegen_spirv/src/abi.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,80 @@ fn trans_struct_or_union<'tcx>(
740740
}
741741
};
742742
}
743+
744+
// The loop above only emits a multi-variant `enum`'s tag, not its variant
745+
// payloads. `rustc_codegen_ssa` accesses payload fields by byte offset
746+
// (`inbounds_ptradd` after a downcast), so for `BackendRepr::Memory` `enum`s
747+
// the payload fields must be present to recover a valid `OpAccessChain`,
748+
// otherwise it's zombie'd as "cannot offset a pointer to an arbitrary element".
749+
//
750+
// So also emit each variant's non-ZST payload fields at their offsets,
751+
// skipping any that overlap an already-present field (struct fields can't
752+
// overlap). This should eventually be replaced with untyped memory + `qptr`.
753+
if union_case.is_none()
754+
&& let Variants::Multiple { variants, .. } = &ty.variants
755+
{
756+
// Track the byte ranges already covered, to reject overlapping payloads.
757+
let mut covered: Vec<(Size, Size)> = field_offsets
758+
.iter()
759+
.zip(&field_types)
760+
.map(|(&offset, &field_ty)| {
761+
let end = cx
762+
.lookup_type(field_ty)
763+
.sizeof(cx)
764+
.map_or(offset, |size| offset + size);
765+
(offset, end)
766+
})
767+
.collect();
768+
769+
let mut extra = Vec::new();
770+
for variant_idx in variants.indices() {
771+
let variant = ty.for_variant(cx, variant_idx);
772+
for i in variant.fields.index_by_increasing_offset() {
773+
let field = variant.field(cx, i);
774+
if field.is_zst() {
775+
continue;
776+
}
777+
let offset = variant.fields.offset(i);
778+
let end = offset + field.size;
779+
if covered.iter().any(|&(o, e)| offset < e && o < end) {
780+
continue;
781+
}
782+
covered.push((offset, end));
783+
let name = match ty.ty.kind() {
784+
TyKind::Adt(adt, _) => {
785+
adt.variants()[variant_idx].fields[FieldIdx::new(i)].name
786+
}
787+
_ => Symbol::intern(&format!("variant{}_field{i}", variant_idx.as_usize())),
788+
};
789+
extra.push((offset, field.spirv_type(span, cx), name));
790+
}
791+
}
792+
793+
// Merge in the payload fields, sorted by offset so the tag stays field
794+
// `0` (required by `recover_access_chain_from_offset` and discriminant
795+
// access).
796+
if !extra.is_empty() {
797+
let mut merged: Vec<_> = field_offsets
798+
.iter()
799+
.zip(&field_types)
800+
.zip(&field_names)
801+
.map(|((&offset, &field_ty), &name)| (offset, field_ty, name))
802+
.collect();
803+
merged.extend(extra);
804+
merged.sort_by_key(|&(offset, ..)| offset);
805+
806+
field_offsets.clear();
807+
field_types.clear();
808+
field_names.clear();
809+
for (offset, field_ty, name) in merged {
810+
field_offsets.push(offset);
811+
field_types.push(field_ty);
812+
field_names.push(name);
813+
}
814+
}
815+
}
816+
743817
SpirvType::Adt {
744818
def_id: def_id_for_spirv_type_adt(ty),
745819
size,
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
#![crate_name = "issue_615"]
2+
3+
// Tests the generated SPIR-V type shapes for multi-variant `enum`s whose layout
4+
// is `BackendRepr::Memory` (i.e. not niche- or `ScalarPair`-optimized). Their
5+
// variant payloads must appear as fields of the `OpTypeStruct`, otherwise field
6+
// accesses into a payload can't be codegen'd into a valid `OpAccessChain` (which
7+
// is what regressed in #615, where `Result<u32, TryFromIntError>` became such an
8+
// `enum` once `TryFromIntError` stopped being a ZST).
9+
//
10+
// Here the payloads sit at distinct offsets, so every payload is exposed:
11+
// `Multi` becomes `{ discriminant, u32, u32 }` and `Tri` becomes
12+
// `{ discriminant, u32, u32, u32 }`.
13+
14+
// build-pass
15+
// compile-flags: -C llvm-args=--disassemble-globals
16+
// normalize-stderr-test "([A-Za-z]:)?/\S*/library/core/src/" -> "$$CORE_SRC/"
17+
// normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> ""
18+
// normalize-stderr-test "OpSource .*\n" -> ""
19+
// normalize-stderr-test "OpExtension .SPV_KHR_vulkan_memory_model.\n" -> ""
20+
// normalize-stderr-test "OpMemoryModel Logical Vulkan" -> "OpMemoryModel Logical Simple"
21+
22+
// `compiletest` handles `ui\dis\`, but not `ui\\dis\\`, on Windows.
23+
// normalize-stderr-test "ui/dis/" -> "$$DIR/"
24+
25+
use spirv_std::spirv;
26+
27+
// `B`'s two payload fields force `BackendRepr::Memory` (no single common
28+
// primitive across variants) and sit at distinct offsets, so both are exposed
29+
// alongside the discriminant.
30+
pub enum Multi {
31+
A(u32),
32+
B(u32, u32),
33+
}
34+
35+
pub enum Tri {
36+
A(u32),
37+
B(u32, u32),
38+
C(u32, u32, u32),
39+
}
40+
41+
#[inline(never)]
42+
fn read_multi(e: &Multi) -> u32 {
43+
match e {
44+
Multi::A(x) => *x,
45+
Multi::B(_, b) => *b,
46+
}
47+
}
48+
49+
#[inline(never)]
50+
fn read_tri(e: &Tri) -> u32 {
51+
match e {
52+
Tri::A(x) => *x,
53+
Tri::B(_, b) => *b,
54+
Tri::C(_, _, c) => *c,
55+
}
56+
}
57+
58+
#[spirv(fragment)]
59+
pub fn main(#[spirv(flat)] i: u32, out: &mut u32) {
60+
let m = if i > 0 { Multi::A(i) } else { Multi::B(1, 2) };
61+
let t = if i > 1 {
62+
Tri::A(i)
63+
} else if i == 1 {
64+
Tri::B(1, 2)
65+
} else {
66+
Tri::C(1, 2, 3)
67+
};
68+
*out = read_multi(&m).wrapping_add(read_tri(&t));
69+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
OpCapability Shader
2+
OpMemoryModel Logical Simple
3+
OpEntryPoint Fragment %1 "main" %2 %3
4+
OpExecutionMode %1 OriginUpperLeft
5+
%4 = OpString "$CORE_SRC/num/mod.rs"
6+
%5 = OpString "$DIR/issue-615.rs"
7+
OpName %6 "Multi"
8+
OpMemberName %6 0 "discriminant"
9+
OpMemberName %6 1 "0"
10+
OpMemberName %6 2 "1"
11+
OpName %7 "Tri"
12+
OpMemberName %7 0 "discriminant"
13+
OpMemberName %7 1 "0"
14+
OpMemberName %7 2 "1"
15+
OpMemberName %7 3 "2"
16+
OpName %2 "i"
17+
OpName %3 "out"
18+
OpName %8 "issue_615::read_multi"
19+
OpName %9 "issue_615::read_tri"
20+
OpDecorate %2 Flat
21+
OpDecorate %2 Location 0
22+
OpDecorate %3 Location 0
23+
%10 = OpTypeInt 32 0
24+
%11 = OpTypePointer Input %10
25+
%12 = OpTypePointer Output %10
26+
%13 = OpTypeVoid
27+
%14 = OpTypeFunction %13
28+
%6 = OpTypeStruct %10 %10 %10
29+
%15 = OpTypePointer Function %6
30+
%7 = OpTypeStruct %10 %10 %10 %10
31+
%16 = OpTypePointer Function %7
32+
%2 = OpVariable %11 Input
33+
%17 = OpTypeBool
34+
%18 = OpConstant %10 0
35+
%19 = OpTypePointer Function %10
36+
%20 = OpConstant %10 1
37+
%21 = OpConstant %10 2
38+
%22 = OpConstant %10 3
39+
%23 = OpTypeFunction %10 %15
40+
%24 = OpTypeInt 32 1
41+
%25 = OpConstant %24 0
42+
%26 = OpTypeFunction %10 %16
43+
%27 = OpUndef %10
44+
%3 = OpVariable %12 Output
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![crate_name = "issue_615"]
2+
3+
// Tests that fallible integer conversions through `TryFrom`/`Result` can be
4+
// codegen'd. This regressed after the nightly-2026-05-22 toolchain upgrade with
5+
// errors like "cannot cast between pointer types" and "cannot offset a pointer
6+
// to an arbitrary element" originating from `core::convert::num`.
7+
//
8+
// The root cause: `core::num::TryFromIntError` is no longer a ZST (it carries a
9+
// niche-bearing error-kind `enum`), so `Result<u32, TryFromIntError>` is laid
10+
// out as `BackendRepr::Memory` with its `Ok`/`Err` payloads at distinct offsets.
11+
// Such multi-variant `enum`s previously had their payloads omitted from the
12+
// generated SPIR-V type, breaking payload field accesses.
13+
14+
// build-pass
15+
16+
use spirv_std::spirv;
17+
18+
#[spirv(fragment)]
19+
pub fn main(#[spirv(flat)] i: u32, out: &mut u32) {
20+
let v = i as usize;
21+
22+
// Direct conversion via `TryFrom` + `Result`/`Option` adapters.
23+
let mut acc = u32::try_from(v).ok().unwrap_or(0);
24+
25+
// The `Result<u32, TryFromIntError>` also shows up indirectly inside
26+
// `StepBy`'s iteration, which is the form originally reported in #615.
27+
for index in (0..v).step_by(2) {
28+
acc = acc.wrapping_add(index as u32);
29+
}
30+
31+
*out = acc;
32+
}

0 commit comments

Comments
 (0)