EmbarkStudios / rust-gpu

🐉 Making Rust a first-class language and ecosystem for GPU shaders 🚧
https://shader.rs
Apache License 2.0
7.35k stars 245 forks source link

For loops need more optimizations #739

Open hrydgard opened 3 years ago

hrydgard commented 3 years ago

Due to how for loops over a range work in Rust, a lot of scaffolding code is generated that is then expected to be optimized away, but in rust-gpu, that doesn't happen enough yet. We had a case where I had to change a for i in 0..8 {} to a while i < 8 { ...; i++ } loop to work around SPIRV-Cross making a right mess out of it.

Filing this issue to have something to point to from the code comment, mainly, not expectations for a quick resolution as I know it's a thorny one.

khyperia commented 3 years ago

Some repro steps: this rust-gpu program

#![cfg_attr(
    target_arch = "spirv",
    no_std,
    feature(register_attr, lang_items),
    register_attr(spirv)
)]

#[cfg(not(target_arch = "spirv"))]
use spirv_std::macros::spirv;

use spirv_std as _;

#[spirv(fragment)]
pub fn main_fs(input: i32, output: &mut i32) {
    let mut result = 0;
    for _ in 0..10 {
        result += input;
    }
    *output = result;
}

when compiled and ran through spirv-cross -V (compile to GLSL), spits out this GLSL:

#version 450

struct _14
{
    uint _m0;
    int _m1;
};

struct _15
{
    int _m0;
    int _m1;
};

layout(location = 0) in int _2;
layout(location = 0) out int _3;

_14 _26;
int _31;

void main()
{
    bool _76 = false;
    do
    {
        _15 _33;
        _33._m0 = 0;
        _33._m1 = 10;
        _14 _41;
        _41 = _26;
        int _45;
        _14 _42;
        bool _79;
        int _44 = 0;
        for (;;)
        {
            if (_33._m0 < _33._m1)
            {
                int _34;
                _34 = _33._m0;
                int _35 = _33._m0 + int(1u);
                _33._m0 = _35;
                _42 = _14(1u, _34);
            }
            else
            {
                _14 _65 = _41;
                _65._m0 = 0u;
                _42 = _65;
            }
            bool _55_ladder_break = false;
            switch (int(_42._m0))
            {
                case 0:
                {
                    _3 = _44;
                    _76 = true;
                    _79 = true;
                    _55_ladder_break = true;
                    break;
                }
                default:
                {
                    _79 = false;
                    _55_ladder_break = true;
                    break;
                }
            }
            if (_55_ladder_break)
            {
                break;
            }
            _45 = _44 + _2;
            _41 = _42;
            _44 = _45;
            continue;
        }
        if (_79)
        {
            break;
        }
        break;
    } while(false);
}

note how _45 = _44 + _2; is dead code! (due to _55_ladder_break always being true)

khyperia commented 3 years ago

I've filed a SPIRV-Cross issue at https://github.com/KhronosGroup/SPIRV-Cross/issues/1731 - unfortunately I wasn't able to narrow down the repro any more, hopefully something actionable still comes out of it :(