mojoshader icon indicating copy to clipboard operation
mojoshader copied to clipboard

[spirv][glspirv] Const Arrays

Open kg opened this issue 5 years ago • 11 comments

Just a note for posterity that, like in every other backend before it, the spir-v backend seems to have a partially or completely broken implementation for local const arrays. static const arrays might also be broken but i haven't tested them.

Example problem code snippet:

#define DEFINE_QuadCorners const float2 QuadCorners[] = { \
    {0, 0}, \
    {1, 0}, \
    {1, 1}, \
    {0, 1} \
};

inline float2 ComputeCorner (
    in int2 cornerIndex : BLENDINDICES0,
    in float2 regionSize
) {
    DEFINE_QuadCorners
    float2 corner = QuadCorners[cornerIndex.x];
    return corner * regionSize;
}

in original XNA along with FNA3D d3d11 and FNA3D glsl, QuadCorners[1] will be {1, 0} but in spir-v right now, it seems to always be 0.

I'm just going to learn my lesson and never use arrays again but maybe someone else will run into this problem in the future.

kg avatar Aug 29 '20 19:08 kg

Pretty sure this is the relevant block:

https://github.com/FNA-XNA/MojoShader/blob/upstream/profiles/mojoshader_profile_spirv.c#L2345-L2454

It should be emitting constants, vs ARB_gl_spirv where it emits constants as uniform data.

flibitijibibo avatar Aug 29 '20 20:08 flibitijibibo

More context for testing/troubleshooting, since I ran into another bit of code (not mine, this time) that uses them. Neither of these snippets work in Vulkan:

static const float thresholdMatrix[] = {
    1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
    13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
    4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
    16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
};

bool StippleReject (float index, float stippleFactor) {
    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

nor this:

bool StippleReject (float index, float stippleFactor) {
    const float thresholdMatrix[] = {
        1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
        13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
        4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
        16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
    };

    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

kg avatar Aug 30 '20 08:08 kg

Bad news is that /mojoshaderprofile:glspirv is not broken. The good news is that uniform const float thresholdMatrix[] does work, so I can use that until we figure this out. (Maybe I should just use that in general?)

kg avatar Aug 30 '20 08:08 kg

The workaround seems fine, but this tells me the block I linked to is right - the only difference between the GL and VK modes are the VPOS fixup and the const array emit block.

flibitijibibo avatar Aug 30 '20 15:08 flibitijibibo

It will still be a few days until I fully catch up with mojoshader and fna3d development after summer madness. I will try to build some test shaders when I get around to it to see what dxbc looks like for them. I don't see any obvious reason why it would work with glspirv (in GL), but not with vkspirv (in vulkan).

Is there difference between dxbc for local and global const array? Also, who fills in values? Should those be baked into the shader or is it done by preshader?

krolli avatar Sep 09 '20 22:09 krolli

I've been experimenting with those arrays, but nothing obviously wrong comes to mind. Test shaders I used were compiled with fxc.exe /nologo /T ps_3_0 /O3 "hlsl\array-1-global.hlsl" /Fo "fxb\array-1-global.psa" /Fc "fxb\array-1-global.psh". Attaching them for reference:

static const float thresholdMatrix[] = {
    1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
    13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
    4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
    16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
};

bool StippleReject (float index, float stippleFactor) {
    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

void main(
    in  float4 in0  : TEXCOORD0,
    out float4 out0 : COLOR0
)
{
    out0 = StippleReject(in0.x, in0.y) ? float4(0, 0, 0, 0) : float4(1, 1, 1, 1);
}
bool StippleReject (float index, float stippleFactor) {
    const float thresholdMatrix[] = {
        1.0 / 17.0,  9.0 / 17.0,  3.0 / 17.0, 11.0 / 17.0,
        13.0 / 17.0,  5.0 / 17.0, 15.0 / 17.0,  7.0 / 17.0,
        4.0 / 17.0, 12.0 / 17.0,  2.0 / 17.0, 10.0 / 17.0,
        16.0 / 17.0,  8.0 / 17.0, 14.0 / 17.0,  6.0 / 17.0
    };

    float stippleThreshold = thresholdMatrix[index % 16];
    return (stippleFactor - stippleThreshold) <= 0;
}

void main(
    in  float4 in0  : TEXCOORD0,
    out float4 out0 : COLOR0
)
{
    out0 = StippleReject(in0.x, in0.y) ? float4(0, 0, 0, 0) : float4(1, 1, 1, 1);
}
#define DEFINE_QuadCorners const float2 QuadCorners[] = { \
    {0, 0}, \
    {1, 0}, \
    {1, 1}, \
    {0, 1} \
};

inline float2 ComputeCorner (
    in int2 cornerIndex : BLENDINDICES0,
    in float2 regionSize
) {
    DEFINE_QuadCorners
    float2 corner = QuadCorners[cornerIndex.x];
    return corner * regionSize;
}

void main(
    in  float2 in0  : TEXCOORD0,
    in  int2   in1  : BLENDINDICES0,
    out float4 out0 : COLOR0
)
{
    out0 = float4(0,0,0,1);
    out0.xy = ComputeCorner(in1, in0);
}

Unfortunately, fxc spits out exactly same binary for global and local array in StippleReject. Also, glspirv and vkspirv results are exactly the same for all three shaders.

@kg, can you provide compiled versions of any of those problematic shaders? Ideally version that works and one that doesn't so I can see what is different between them.

It is possible that the way constants are emitted for SPIR-V is only compatible with OpenGL and not with Vulkan, but that would probably show up elsewhere as well, so I'm not convinced that is the case.

Ethan might be right that the problem stems from uniform handling, though I'd like to be sure before making changes in it.

krolli avatar Sep 12 '20 06:09 krolli

The global and local array should produce the same binary, the thing that works is uniform const float x[] at the top level.

kg avatar Sep 12 '20 17:09 kg

Ah, right, you said that neither of them works. In any case, those shaders I posted compile into what looks like perfectly normal binary. Unfortunately, fxc replaced array access in these simple shaders with a bunch of comparison tricks and simple constants (not uniforms).

krolli avatar Sep 12 '20 22:09 krolli

I found some minor difference between SPIR-V output of glslangValidator and mojoshader. While mojoshader uses initializer operand of OpVariable instruction to set constant data, glslangValidator emits separate OpStore instruction in main entrypoint. It's a shot in the dark, but maybe it will fix the problem. I'm not sure whether this is bug in the driver or our generated code though. If it fixes the problem, I'll try to go over the spec in more detail to see if there is some mention this.

Here's the commit, if you could give it a try and see if it helps: https://github.com/krolli/MojoShader/commit/06f924f1e0efe625b508a5cd1be7a26777785970

krolli avatar Sep 13 '20 06:09 krolli

@kg, have you tried those changes to see if they help with the problem?

krolli avatar Nov 07 '20 10:11 krolli

Missed the previous comment, will try to take a look soon

kg avatar Nov 07 '20 23:11 kg