Skip to content
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

Allow using MaybeUninit #981

Open
NiklasNummelin opened this issue Jan 3, 2023 · 5 comments
Open

Allow using MaybeUninit #981

NiklasNummelin opened this issue Jan 3, 2023 · 5 comments
Labels
t: enhancement A new feature or improvement to an existing one.

Comments

@NiklasNummelin
Copy link

NiklasNummelin commented Jan 3, 2023

When using MaybeUninit to initialize variables one gets this issue:
error: Cannot cast between pointer types. From: [u8; 256]. To: *struct core::mem::ManuallyDrop<[Vec4; 16]> { value: [f32x4; 16] }.

Would be nice to allow, if possible, as it's often unnecessary to initialize variables in shaders and adds additional overhead.

@NiklasNummelin NiklasNummelin added the t: enhancement A new feature or improvement to an existing one. label Jan 3, 2023
@NiklasNummelin
Copy link
Author

I can also add that using core::mem::uninitialized() leads to the same issue.

@LegNeato
Copy link
Contributor

Hmmm, from the tests it looks like this might already be supported?

https://github.com/EmbarkStudios/rust-gpu/blob/main/tests/ui/lang/core/mem/create_unitialized_memory.rs

@eddyb
Copy link
Contributor

eddyb commented Mar 17, 2023

@NiklasNummelin @LegNeato Sorry I forgot about this, but it looks like the issue here is specifically arrays?

What were you using MaybeUninit for? It looks like .as_mut_ptr() is fine for inline asm! use and we should replace all our indirect-asm!-result patterns with it.

EDIT: oh, I see, MaybeUninit<T> works already when T is an integer/float/pointer, because it bypasses the general case, will still need to implement the general case.

@eddyb
Copy link
Contributor

eddyb commented Mar 17, 2023

This PR has a fix (in its first commit) for unions that should handle all possible MaybeUninit<T>s:

However, I haven't added tests, or even tried it out beyond the spirv_std changes, so even after that lands, this issue would be tracking making sure @NiklasNummelin's original usecase is addressed.

@eddyb
Copy link
Contributor

eddyb commented Mar 22, 2023

Haven't looked into reproducing this yet but it's very likely that MaybeUninit<&[T]> (and scalar pairs in general) has a different layout than &[T], which seems to be negatively impacting #1014.

It might be fine for fixed-length arrays, so your original usecase is probably fine, but TypedBuffer kind of needs to work with slices to be maximally useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: enhancement A new feature or improvement to an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants
@eddyb @NiklasNummelin @LegNeato and others