KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

How binding points should be assigned when some array instances are eliminated by optimizer? #46

Closed asimiklit closed 5 years ago

asimiklit commented 5 years ago

I am working on a mesa issue: https://bugs.freedesktop.org/show_bug.cgi?id=109532

This issue happens because the mesa assigns unexpected for a deqp test the binding points for an ssbo array instances.

The issue explanation: We have the following compute shader:

#version 430

   layout(local_size_x = 1) in;

   layout(packed, binding = 0) buffer BlockA
   {
           float a;
   } blockA[6];

   void main (void)
   {
       blockA[2].a = 77777.77777;
       blockA[4].a = 88888.88888;
   }

After compilation and linking mesa assigns the following binding points: BlockB[0] is eliminated BlockB[1] is eliminated BlockB[2] is associated with binding point 0 BlockB[3] is eliminated BlockB[4] is associated with binding point 1 BlockB[5] is eliminated

But deqp test expects the following binding points: BlockB[0] is eliminated BlockB[1] is eliminated BlockB[2] is associated with binding point 2 BlockB[3] is eliminated BlockB[4] is associated with binding point 4 BlockB[5] is eliminated

I find the following information which relates to this question:

ARB_shader_storage_buffer_object:

   (1) "(modify second paragraph, p. 59) If the <binding> identifier is used with
        a uniform or shader storage block instanced as an array then the first
        element of the array takes the specified block binding and each subsequent
        element takes the next consecutive block binding point."
   (2) "(modify third paragraph, p. 59) If the binding point for any uniform or
        shader storage block instance is less than zero or greater than or equal
        to the implementation-dependent maximum number of bindings for the block
        type (uniform or shader storage), a compilation error will occur.  When
        the binding identifier is used with a uniform or shader storage block
        instanced as an array of size <N>, all elements of the array from
        <binding> through <binding>+<N>-1 must be within this range."
   (3) "Do we allow arrays of shader storage blocks?

        RESOLVED:  Yes; we already allow arrays of uniform blocks, where each
        block instance has an identical layout but is backed by a separate
        buffer object.  It seems like we should do this here for consistency.

        If we had overloaded the existing uniform block APIs (e.g., by applying
        a "readwrite" layout qualifier to uniform blocks), it would be really
        weird if we disallowed arrays of writeable uniform blocks since we
        already allow it for regular (read-only) uniform blocks."

OpenGL 4.6 (Core Profile):

   (4) "Like buffer variables, shader storage blocks can be active or inactive. Whether
        a shader storage block is active or inactive is determined in the same way as for
        uniform blocks (see section 7.6). Additionally though, all members of a named
        shader storage block declared with a std430 layout qualifier are considered active,
        even if they are not referenced in any shader in the program."

Looks like the current mesa implementation satisfy for (2) requirements. But according to (1) the mesa is wrong because the BlockB[0] should be associated with binding point 0, on the other hand according to (3) and (4) it able to deactivate BlockB[0] due to it unuse. So what happens with the binding point in this case, can we reuse it for the other instances of this array or not...

Could you please explain how the binding points should be assigned in this case?

pdaniell-nv commented 5 years ago

I believe all buffer binding points should be consumed, regardless whether the array elements are used or not. This would be the behavior of least surprise to the developer. I didn't see any language that would indicate that unused elements should not be counted when assigning the element to the buffer binding point.

imirkin commented 5 years ago

There's a slight corollary to this question... what if you had code like

layout(packed) buffer BlockA { float a; } blockA[6];

And then used elements 0 and 5 in the shader. And the program looked up the binding point for BlockA[0] -- would it be reasonable for the program to assume that BlockA[5]'s binding point is BlockA[0] + 5?

Basically ... should we only be doing this when there's an explicit binding location supplied, or anytime any part of the interface array is used?

ianromanick commented 5 years ago

The third question is, in either the explicit or implicit case, can the bindings associated with the unused elements be used for other things? In the layout(packed, binding=0) case, could binding 1 be used for something else?

ianromanick commented 5 years ago

But deqp test expects the following binding points: BlockB[0] is eliminated BlockB[1] is eliminated BlockB[2] is associated with binding point 2 BlockB[3] is eliminated BlockB[4] is associated with binding point 4 BlockB[5] is eliminated

I think this expectation is definitely valid. Anything else is madness. The application has statically assigned these locations, and it has expectations built into its code. The GLSL compiler / linker may be able to determine that some array element is unused based on advanced optimizations... changing the bindings of the remaining elements can only cause problems.

I think I've answered my own previous question. If an application is built this way, it may just bind buffers to all the bindings for the block array. Reusing one of the intermediate binding points can only cause problems. Say we eliminate BlockB[1] and assign its location to some other block. Whether or not the application gets correct behavior depends on the order they bind buffers to that location. Madness.

pdaniell-nv commented 5 years ago

We discussed this in the OpenGL/ES working group meeting and agreed that eliminating unused elements from the interface block array is not desirable. There is no statement in the spec that this takes place and it would be highly implementation dependent if it happens. If the application has an "interface" in the shader they need to match up with the API it would be quite confusing to have the binding point get compacted. So the answer is no, the binding points aren't affected by unused elements in the interface block array.