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

Missing ability to qualify struct members as Restrict? #97

Open
haasn opened this issue Mar 1, 2021 · 5 comments
Open

Missing ability to qualify struct members as Restrict? #97

haasn opened this issue Mar 1, 2021 · 5 comments

Comments

@haasn
Copy link

haasn commented Mar 1, 2021

GLSL spec says:

The memory qualifiers coherent, volatile, restrict, readonly, and writeonly may be used in the declaration of buffer variables (i.e., members of shader storage blocks). When a buffer variable is declared with a memory qualifier, the behavior specified for memory accesses involving image variables described above applies identically to memory accesses involving that buffer variable. It is a compile-time error to assign to a buffer variable qualified with readonly or to read from a buffer variable qualified with writeonly.

SPIR-V spec says:

Restrict
Apply only to a memory object declaration, to indicate the compiler may compile as if there is no aliasing. See the Aliasing section for more detail.

Possibly relevant, from the change-log:

Khronos SPIR-V Issue #408: (Re)allow the decorations Volatile, Coherent, NonWritable, and NonReadable on members of blocks. (Temporarily dropping this functionality was accidental/clerical; intent is that it has always been present.)

Was the intent to continue disallowing Restrict on members of blocks?

It's worth pointing out that glslang accepts GLSL code with such declarations, and turns them into what is technically invalid SPIR-V:

e.g.

layout(std430, binding=1) coherent restrict buffer _PeakDetect_2_0 {
    layout(offset=0) vec2 average;
    layout(offset=8) int frame_sum;
    layout(offset=12) int frame_max;
    layout(offset=16) uint counter;
};

becomes:

               OpName %_PeakDetect_2_0 "_PeakDetect_2_0"
               OpMemberName %_PeakDetect_2_0 0 "average"
               OpMemberName %_PeakDetect_2_0 1 "frame_sum"
               OpMemberName %_PeakDetect_2_0 2 "frame_max"
               OpMemberName %_PeakDetect_2_0 3 "counter"
...
               OpMemberDecorate %_PeakDetect_2_0 0 Coherent
               OpMemberDecorate %_PeakDetect_2_0 0 Restrict
               OpMemberDecorate %_PeakDetect_2_0 0 Offset 0
               OpMemberDecorate %_PeakDetect_2_0 1 Coherent
               OpMemberDecorate %_PeakDetect_2_0 1 Restrict
               OpMemberDecorate %_PeakDetect_2_0 1 Offset 8
               OpMemberDecorate %_PeakDetect_2_0 2 Coherent
               OpMemberDecorate %_PeakDetect_2_0 2 Restrict
               OpMemberDecorate %_PeakDetect_2_0 2 Offset 12
               OpMemberDecorate %_PeakDetect_2_0 3 Coherent
               OpMemberDecorate %_PeakDetect_2_0 3 Restrict
               OpMemberDecorate %_PeakDetect_2_0 3 Offset 16
               OpDecorate %_PeakDetect_2_0 Block

Which mesa's SPIRV-to-NIR compiler, in turn, rejects as invalid:

SPIR-V WARNING:
    In file ../src/compiler/spirv/spirv_to_nir.c:1073
    Decoration not allowed on struct members: SpvDecorationRestrict
    1552 bytes into the SPIR-V binary
@haasn
Copy link
Author

haasn commented Mar 1, 2021

Also worth pointing out that SPIRV-Cross takes the above (invalid) SPIR-V and reflects it back into (valid) GLSL:

layout(set = 0, binding = 1, std430) coherent restrict buffer _PeakDetect_2_0
{
    vec2 average;
    int frame_sum;
    int frame_max;
    uint counter;
} _161;

@Hugobros3
Copy link

Hugobros3 commented Mar 5, 2021

Sounds like a bug in mesa to me. I don't see how you conclude the spec disallows restrict, it clearly allows it, GLSL buffers get translated to an OpVariable containing a OpTypePointer that is thus a "memory object delcaration" and may include those Restrict annotations.

Also the spec later says:

The Simple, GLSL, and Vulkan memory models can assume that aliasing is generally not present between the memory
object declarations [ ... ] Applying Restrict is allowed, but has no effect.

@gfxstrand
Copy link
Contributor

Sounds like a bug in mesa to me. I don't see how you conclude the spec disallows restrict, it clearly allows it, GLSL buffers get translated to an OpVariable containing a OpTypePointer that is thus a "memory object delcaration" and may include those Restrict annotations.

Also the spec later says:

The Simple, GLSL, and Vulkan memory models can assume that aliasing is generally not present between the memory
object declarations [ ... ] Applying Restrict is allowed, but has no effect.

That bit of spect text is in reference to memory object declarations, not to structure members. It's saying that Restrict has no effect because it's the default. That doesn't mean you can put it anywhere you please when the spec for the Restrict qualifier explicitly says otherwise.

@Hugobros3
Copy link

Hugobros3 commented Mar 5, 2021

I see, the problem is you can have Restrict on the pointer type for the OpVariable because that's a mem object declaration, but not on the members themselves. So basically #93 again, and a bug in glslang ?

@GeorgeS2019
Copy link

GeorgeS2019 commented Apr 3, 2022

I was suggested to share the problem with WSL2 running llvmpipe here

If anyone has idea what problem I face here and how to address that. Appreciate!

Using present mode: VK_PRESENT_MODE_FIFO_KHR
Using present mode: VK_PRESENT_MODE_FIFO_KHR
SPIR-V WARNING:
    In file ../src/compiler/spirv/spirv_to_nir.c:1069
    Decoration not allowed on struct members: SpvDecorationRestrict
    4412 bytes into the SPIR-V binary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants