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

D3DCompile error with vector indexing #4460

Open
Noxime opened this issue Jan 27, 2022 · 6 comments
Open

D3DCompile error with vector indexing #4460

Noxime opened this issue Jan 27, 2022 · 6 comments
Labels
area: naga back-end Outputs of naga shader conversion lang: HLSL D3D Shading Language naga Shader Translator type: bug Something isn't working

Comments

@Noxime
Copy link
Contributor

Noxime commented Jan 27, 2022

When trying to assign to a vector component through indexing, the Direct3D shader compiler rejects the generated HLSL. On Vulkan and Metal the shader is built without any issues and runs correctly.

Error message:

wgpu::backend::direct: Shader translation error for stage FRAGMENT: D3DCompile error (0x80004005): C:\Users\Aarop\repos\crucible\shaders\shading.wgsl(59,13-22): warning X3550: array reference cannot be used as an l-value; not natively addressable, forcing loop to unroll
C:\Users\Aarop\repos\crucible\shaders\shading.wgsl(76,5-14): error X3500: array reference cannot be used as an l-value; not natively addressable

Related WGSL (32-bit quaternion decompression)

fn decomp_quat(compressed: u32) -> vec4<f32> {
    var comp = compressed;
    var q = vec4<f32>(0.0);
    let mask = (1u << 9u) - 1u;
    let sqrt1_2 = 0.707106781;

    var i_largest = comp >> 30u;
    var sum_squares = 0.0;

    for (var i: i32 = 3; i >= 0; i = i - 1) {
        if (u32(i) != i_largest) {
            let mag = comp & mask;
            let negbit = (comp >> 9u) & 1u;
            comp = comp >> 10u;
            q[i] = sqrt1_2 * f32(mag) / f32(mask);
            if (negbit == 1u) {
                q[i] = -q[i];
            }
            sum_squares = sum_squares + q[i] * q[i];
        }
    }

    q[i_largest] = sqrt(1.0 - sum_squares);
    return q;
}

Generated HLSL

float4 decomp_quat(uint compressed)
{
    uint comp = (uint)0;
    float4 q = (float4)0;
    uint i_largest = (uint)0;
    float sum_squares = 0.0;
    int i = 3;

    comp = compressed;
    q = float4(0.0.xxxx);
    uint mask = ((1u << 9u) - 1u);
    uint _expr17 = comp;
    i_largest = (_expr17 >> 30u);
    bool loop_init = true;
    while(true) {
        if (!loop_init) {
        int _expr28 = i;
        i = (_expr28 - 1);
        }
        loop_init = false;
        int _expr25 = i;
        if ((_expr25 >= 0)) {
        } else {
            break;
        }
        int _expr31 = i;
        uint _expr33 = i_largest;
        if ((uint(_expr31) != _expr33)) {
            uint _expr35 = comp;
            uint mag = (_expr35 & mask);
            uint _expr37 = comp;
            uint negbit = ((_expr37 >> 9u) & 1u);
            uint _expr42 = comp;
            comp = (_expr42 >> 10u);
            int _expr45 = i;
            q[_expr45] = ((0.7071067690849304 * float(mag)) / float(mask));
            if ((negbit == 1u)) {
                int _expr53 = i;
                int _expr55 = i;
                float _expr57 = q[_expr55];
                q[_expr53] = -_expr57;
            }
            float _expr59 = sum_squares;
            int _expr60 = i;
            float _expr62 = q[_expr60];
            int _expr63 = i;
            float _expr65 = q[_expr63];
            sum_squares = (_expr59 + (_expr62 * _expr65));
        }
    }
    uint _expr68 = i_largest;
    float _expr71 = sum_squares;
    q[_expr68] = sqrt((1.0 - _expr71));
    float4 _expr74 = q;
    return _expr74;
}

From reading the error messages, it seems that the first dynamic indexes in the loop can be avoided by unrolling the loop, but the last access towards the end of the function cannot be unrolled since it is not in a loop and causes the issue. Where does WGSL / WebGPU stand on the issue? Is assignment into vectors by dynamic indexing supported or am I invoking unvalidated behaviour?

Update:
If I wrap the last indexing into a dummy loop of for i in 0..4 { if i == largest_i { q[i] = ... } } then the shader compiler is able to work around by unrolling. Not ideal but if someone else runs into this, it might be useful as a temporary fix

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Jan 27, 2022

As far as im aware, webgpu doesn't allow dynamic indexing into vectors like that. If you were to define it as an array (var q = array<f32, 4>) you could index into it however you need.

I'm totally wrong, see the 5th row here: https://gpuweb.github.io/gpuweb/wgsl/#vector-single-component

This may be a problem with either FXC or dxbc (vs dxil). The workaround using array<f32, 4> will still get around the problem.

@Noxime
Copy link
Contributor Author

Noxime commented Jan 27, 2022

Using an array<f32, 4> for the decompressor works correctly, however naga denies the compressor function if q is changed to an array. "May only be indexed by a constant". WGSL spec says array read access is done with OpCompositeExtract while vector indexing is done with OpVectorExtractDynamic. Fascinating. Is dynamic array read indexing planned or is it purposefully forced to be static? Either way, the workaround you suggested works, thanks!

@kvark
Copy link
Member

kvark commented Jan 27, 2022

WGSL group decided that dynamic indexing for array values needs to be allowed, and SPIR-V based implementation would have to find a way to make it possible. Our implementation isn't updated to reflect that. We had this working at some point, but were happy to remove it once the group previously agreed to disallow that. And then the decision was reversed.

@cwfitzgerald
Copy link
Member

compressor function if q is changed to an array. "May only be indexed by a constant".

Make sure it is stored in a var, not let.

@teoxoy teoxoy added kind: bug area: naga back-end Outputs of naga shader conversion lang: HLSL D3D Shading Language labels Apr 29, 2022
@teoxoy
Copy link
Member

teoxoy commented Feb 24, 2023

Tint had to deal with this as well, see https://bugs.chromium.org/p/tint/issues/detail?id=998.

@jimblandy
Copy link
Member

See also #4337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion lang: HLSL D3D Shading Language naga Shader Translator type: bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

5 participants