Fix union field access by representing unions as structs #269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Draft

LegNeato wants to merge 1 commit into Rust-GPU:main

from LegNeato:issue241

Draft

Fix union field access by representing unions as structs #269

LegNeato wants to merge 1 commit into Rust-GPU:main from LegNeato:issue241

Conversation

LegNeato

Copy link

Collaborator

@LegNeato LegNeato commented Jun 3, 2025

When a union has multiple fields, represent it as a struct with all fields at offset 0 instead of just using the largest field. This allows pointer casts between union fields to work correctly by enabling the recover_access_chain_from_offset function to find valid access chains.

Fixes #241.

@LegNeato LegNeato requested review from eddyb, Firestar99 and schell as code owners

June 3, 2025 20:14

@LegNeato

Copy link

Collaborator Author

LegNeato commented Jun 3, 2025

@simensgreen can you check this fix?

@LegNeato LegNeato force-pushed the issue241 branch from 3818aae to c93f02e Compare

June 3, 2025 20:16

@LegNeato LegNeato changed the title ~~(削除) Fix union field access by representing unions as structs in SPIR-V (削除ここまで)~~ (追記) Fix union field access by representing unions as structs (追記ここまで)

Jun 3, 2025

Firestar99

Firestar99 reviewed

Jun 3, 2025

View reviewed changes

crates/rustc_codegen_spirv/src/abi.rs Show resolved Hide resolved

@LegNeato


 Fix union field access by representing unions as structs

c6c8056

When a union has multiple fields, represent it as a struct with all fields
at offset 0 instead of just using the largest field. This allows pointer
casts between union fields to work correctly by enabling the
recover_access_chain_from_offset function to find valid access chains.
Fixes Rust-GPU#241.

@LegNeato LegNeato force-pushed the issue241 branch from c93f02e to c6c8056 Compare

June 4, 2025 01:52

Firestar99

Firestar99 approved these changes

Jun 4, 2025

View reviewed changes

@simensgreen

Copy link

simensgreen commented Jun 4, 2025

@LegNeato Ok, in this simple example it works, but in a more complicated scenario, it will not.

Now I can reproduce both cannot cast between pointer types and cannot memcpy dynamically sized data, you can check it out in my repo

Full error messages:

error: cannot cast between pointer types
 from `*struct DataMatrix { array1d: [f32; 25], array2d: [[f32; 5]; 5], data_array: [struct Data { a: f32, b: [f32; 3], c: f32 }; 5] }`
 to `*[[f32; 5]; 5]`
 --> shader/src/lib.rs:35:77
 |
 35 | pub fn array2d_ref(&self) -> &[[f32; 5]; 5] { unsafe { &self.array2d } }
 | ^
 |
 note: used from within `<shader::DataMatrix>::array2d_ref`
 --> shader/src/lib.rs:35:77
 |
 35 | pub fn array2d_ref(&self) -> &[[f32; 5]; 5] { unsafe { &self.array2d } }
 | ^
 note: called by `shader::main`
 --> shader/src/lib.rs:50:18
 |
 50 | output[id] = input_matrices[id].array2d_ref()[0];
 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: cannot memcpy dynamically sized data
 --> shader/src/lib.rs:34:54
 |
34 | pub fn array2d(self) -> [[f32; 5]; 5] { unsafe { self.array2d } }
 | ^^^^^^^^^^^^
 |
note: used from within `<shader::DataMatrix>::array2d`
 --> shader/src/lib.rs:34:54
 |
34 | pub fn array2d(self) -> [[f32; 5]; 5] { unsafe { self.array2d } }
 | ^^^^^^^^^^^^
note: called by `shader::main`
 --> shader/src/lib.rs:52:18
 |
52 | output[id] = input_matrices[id].array2d()[0];
 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^