Passing a structure made of two Vec3 to a closure results in miscompilation

[lachlansneff]

Example & Steps To Reproduce

Passing a structure containing two Vec3s to a closure results in the following error message:

error: Pointer operand 435[%435] must be a memory object declaration
    %436 = OpFunctionCall %void %sdf_shader__main_fs___closure_0_ %429 %435

    |
    = note: module "/Users/lachlansneff/dev/sdf/target/spirv-builder/spirv-unknown-unknown/release/deps/sdf_shader.spv"

  warning: internal error
    |
    = note: spirv-opt failed, leaving as unoptimized
    = note: module "/Users/lachlansneff/dev/sdf/target/spirv-builder/spirv-unknown-unknown/release/deps/sdf_shader.spv"

  error: invalid id:0:0 - Pointer operand 435[%435] must be a memory object declaration
    %436 = OpFunctionCall %void %sdf_shader__main_fs___closure_0_ %429 %435

    |
    = note: spirv-val failed
    = note: module "/Users/lachlansneff/dev/sdf/target/spirv-builder/spirv-unknown-unknown/release/deps/sdf_shader.spv"

A way to repro is with the following code:

struct Foo {
    a: Vec3,
    b: Vec3,
}

fn bar(f: impl Fn(Foo)) {
    f(Foo { a: Vec3::zero(), b: Vec3::zero() })
}

#[spirv(fragment)]
pub fn main_fs(_: Output<Vec4>) {
    bar(|_| {});
}

System Info

• Rust: rustc 1.51.0-nightly (257becbfe 2020-12-27)
• OS: macOS 10.15.6

[XAMPPRocky]

Thank you for your issue! Would you also be able to include the full SPIR-V binary? It should be available at /Users/lachlansneff/dev/sdf/target/spirv-builder/spirv-unknown-unknown/release/deps/sdf_shader.spv

You can include it in the issue like so:

<details>
    <summary>SPIR-V</summary>

<pre><code>

<!-- Insert SPIR-V here -->

</code></pre>

</details>

[khyperia]

Thank you for your issue! Would you also be able to include the full SPIR-V binary? It should be available at /Users/lachlansneff/dev/sdf/target/spirv-builder/spirv-unknown-unknown/release/deps/sdf_shader.spv

It's easy enough to build the SPIR-V, this repros it in the test framework:

#[test]
fn fzoo() {
    val(r#"
struct Foo {
    a: glam::Vec3,
    b: glam::Vec3,
}

fn bar(f: impl Fn(Foo)) {
    f(Foo { a: glam::Vec3::zero(), b: glam::Vec3::zero() })
}

#[spirv(fragment)]
pub fn main_fs(_: Output<glam::Vec4>) {
    bar(|_| {});
}"#);
}

It looks like this would likely be fixed by #69 plus tweaks to mem2reg, but I'm not 100% sure. There's some extremely weird codegen going on, with the Foo temporary variable being wrapped in a one-element tuple (Foo,) while it's a local variable (but not when passed into f). TBH it'd be really nice to instead pass structs by value instead of by pointer, but that requires rustc work.

[lachlansneff]

Is there a way to work around this for the time given?

[eddyb]

I can't think of a workaround, if the problem is tupled closure arguments, that's a limitation caused by rustc's broken representation of variadic signatures. I've been tackling this in my spare time lately, so maybe I should try to make more progress on that front.

TBH it'd be really nice to instead pass structs by value instead of by pointer, but that requires rustc work.

OTOH, IIRC this is just the ability to hook FnAbi computation, right? So that might be more doable.

[lachlansneff]

Any progress on this?

[khyperia]

Looks like this was fixed by #690 🎉 - or, well, this updated version of the repro at least (Output<T> -> &mut T, and Vec3::zero() -> Vec3::ZERO)

struct Foo {
    a: Vec3,
    b: Vec3,
}

fn bar(f: impl Fn(Foo)) {
    f(Foo {
        a: Vec3::ZERO,
        b: Vec3::ZERO,
    })
}

#[spirv(fragment)]
pub fn main_fs(_: &mut Vec4) {
    bar(|_| {});
}