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

Support Option<T> #234

Open
fu5ha opened this issue Nov 12, 2020 · 8 comments
Open

Support Option<T> #234

fu5ha opened this issue Nov 12, 2020 · 8 comments
Assignees
Labels
c: rustc_codegen_spirv Issues specific to the rustc_codegen_spirv crate. s: qptr may fix This might be fixed by the "qptr" experiment (https://github.com/EmbarkStudios/spirt/pull/24) t: bug Something isn't working

Comments

@fu5ha
Copy link
Member

fu5ha commented Nov 12, 2020

Expected Behaviour

Be able to use Option<Mat4>

Example & Steps To Reproduce

Use an Option that contains a value larger than some threshold

    pub fn from_mat4(t: &Mat4) -> Option<Self> {
        let (scale3, rotation, translation) = t.to_scale_rotation_translation();
        if scale3.abs_diff_eq(Vec3::splat(1.0), 1e-4) {
            Some(Self::from_rotation_translation(rotation, translation))
        } else {
            None
        }
    }

image

@fu5ha fu5ha added the t: bug Something isn't working label Nov 12, 2020
@fu5ha fu5ha changed the title Large options make rust-gpu sad :( Large Options make rust-gpu sad :( Nov 12, 2020
@fu5ha fu5ha added the c: rustc_codegen_spirv Issues specific to the rustc_codegen_spirv crate. label Nov 12, 2020
@MarijnS95
Copy link
Contributor

@termhn This error is identical to the one I intended to create an issue for following #220 (comment), thanks for waking me up 😄

error: Cannot cast between pointer types
   --> examples/shaders/sky-shader/src/lib.rs:156:19
    |
156 |     let sun_pos = const_vec3!([0.0, 75.0, -1000.0]);
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: from: *{Function} [u8; 16]
    = note: to: *{Function} [f32; 3]
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

Cannot cast between pointer types comes from zombie_bitcast_ptr.

The const_vec3 macro (and the f32x4 in your error likely means a const_quat! somewhere) use a union to perform casting. Using it on a primitive like [ui]64 is fine (for lack of [iu]128):

    union Foo {
        a: u64,
        b: i64,
    }
    let x = unsafe { Foo { a: 1 }.b };

This compiles without errors. Turning it into a statically sized array (internal representation in glam), no matter how small surfaces this error:

    union Foo {
        a: [u8; 1],
        b: [i8; 1],
    }
    let x = unsafe { Foo { a: [1] }.b };
error: Cannot cast between pointer types
   --> examples/shaders/sky-shader/src/lib.rs:163:22
    |
163 |     let x = unsafe { Foo { a: [1] }.b };
    |                      ^^^^^^^^^^^^^^^^
    |
    = note: from: *{Function} [u8; 1]
    = note: to: *{Function} [i8; 1]

Bonus, attempting to "get" a [iu]128 by using a tuple throws something completely different:

    union Foo {
        a: (u64, u64),
        b: (i64, u64),
    }
    let x = unsafe { Foo { a: (1, 1) }.b };
error: Cannot use this pointer directly, it must be dereferenced first
   --> examples/shaders/sky-shader/src/lib.rs:163:22
    |
163 |     let x = unsafe { Foo { a: (1, 1) }.b };
    |                      ^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

I'm not enough of a compiler engineer to take the plunge and understand + solve this issue (despite that I assume these statically sized arrays should be taken and reinterpreted "by value") but this mini-investigation might give someone else a head-start 😁

@khyperia
Copy link
Contributor

Unfortunately these are far from different errors, they merely fail with the same compiler error message. Enums we can fix via the approach that RLSL took and generate an "exploded" data representation, where each enum kind is stored as separate field (and disallow interaction with host-rust). This will remove the need to bitcast pointers, fixing the compiler error reported here.

However, arbitrary bitcasting between pointers is impossible to do on the GPU, and will never be supported.

@XAMPPRocky XAMPPRocky changed the title Large Options make rust-gpu sad :( Support Option<T> Jan 7, 2021
@XAMPPRocky
Copy link
Member

XAMPPRocky commented Jan 7, 2021

Updated title to reflect that this currently affects all Option<T>'s for example this provides the same error.

pub fn foo () -> Option<u8> { Some(5u8) }
error: Cannot cast between pointer types
  --> examples/shaders/sky-shader/src/lib.rs:40:31
   |
40 | pub fn foo () -> Option<u8> { Some(5u8) }
   |                               ^^^^^^^^^
   |
   = note: from: *{Function} struct core::option::Option<u8> { u8, u8 }
   = note: to: *{Function} struct core::option::Option<u8>::Some { 0: u8 }

@XAMPPRocky
Copy link
Member

Adding this as another thing that needs options, which is num_traits::Float::powi, it uses an .unwrap in it's code when converting to usize. https://github.com/rust-num/num-traits/blob/319623616822ab8d77ba0422dc63ed4ff5b9a809/src/float.rs#L684

#[test]
fn powi() {
    val(r#"
fn powi(x: f32) -> f32 {
    x.powi(80)
}
#[allow(unused_attributes)]
#[spirv(fragment)]
pub fn main() {
    assert!(powi(1.0) == 80.0);
}
"#);
}

Error

error: Cannot cast between pointer types
   --> /Users/.cargo/registry/src/github.com-1ecc6299db9ec823/num-traits-0.2.14/src/cast.rs:278:1
    |
278 | impl_to_primitive_uint!(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: from: *{Function} struct core::option::Option<usize> { u32, u32 }
    = note: to: *{Function} struct core::option::Option<usize>::Some { 0: u32 }
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: Cannot cast between pointer types
   --> /Users/.rustup/toolchains/nightly-2020-12-28-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/option.rs:385:18
    |
385 |             Some(val) => val,
    |                  ^^^
    |
    = note: from: *{Function} struct core::option::Option<usize> { u32, u32 }
    = note: to: *{Function} struct core::option::Option<usize>::Some { 0: u32 }

@DavidTruby
Copy link

This issue also seems to prevent num-complex compiling in shader crates. I'd be interested in having a deeper look at this if it's possible to fix. I have a couple of questions though.

Firstly, why is it that pointer bitcasts aren't possible on the GPU? I worked (briefly) on the GPU OpenMP backend in clang and am pretty sure I remember that we allowed arbitrary reinterpret_casts on there.

Secondly, specifically what I'm seeing is pointer casts like from: *[struct &str { *[u8], u32 }; 6] to *[struct &str { *[u8], u32 }]. I'm not really a rust expert, but why are these even considered pointer casts at all from the GPU's perspective? Aren't these just casts from fixed to "variable" length arrays? The CPU architectures I'm familiar with have no concept of fixed length arrays anyway (as far as I'm aware) so surely this cast is a no-op by that point even on a CPU?

If anyone can point me in the right direction here I'd be happy to take a look!

@khyperia
Copy link
Contributor

(meta note: I believe you discussed those questions on discord with eddyb after you posted this, so I won't answer again here - let me know if I'm mistaken and I can give answers here)

@eddyb eddyb added the s: qptr may fix This might be fixed by the "qptr" experiment (https://github.com/EmbarkStudios/spirt/pull/24) label Mar 29, 2023
@eddyb eddyb self-assigned this Mar 31, 2023
@Lucky4Luuk
Copy link

Any updates on this? I'd love to be able to use Option in shaders.

@reinismu
Copy link

EmbarkStudios/spirt#24 doesn't unlock Option?

@rust-gpu-bot
Copy link

This issue is now being tracked at: Rust-GPU/rust-gpu#139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: rustc_codegen_spirv Issues specific to the rustc_codegen_spirv crate. s: qptr may fix This might be fixed by the "qptr" experiment (https://github.com/EmbarkStudios/spirt/pull/24) t: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants