[Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

> You are completely missing the point.  Any test which tests the
> GL_ARB_fragment_shader_interlock extension with a
> beginInvocationInterlockARB() call inside of control-flow would be an
> invalid test for GL_ARB_fragment_shader_interlock since that behavior is
> undefined.  Therefore, any such test would be a bad test.  Unless we have
> tests which test beginFragmentShaderOrderingINTEL() inside non-uniform
> control-flow, it is insufficiently tested.  Therefore, any tests we may
> have for GL_ARB_fragment_shader_interlock are either invalid use of the ARB
> extension or they are insufficient to test the INTEL extension.

My apologies, I misunderstood your question "is that tested?"; I thought the question was about if the assentation that the current implementation of beginFragmentShaderOrderingINTEL() and beginInvocationInterlockARB() were functionally interchangeable has been verified/tested.

FWIW, a test where beginInvocationInterlockARB() is called within any control flow or for that matter not called from main(), the shader is supposed to be rejected according the spec. In a negative way, it is another test. Though, while acknowledging Ian's point that "works for vendor X" has caused a great deal of GLSL fragmentation, I still personally prefer to implementation to try to take what applications throw at them as much as reasonable possible. However, that is not my decision to make and whatever the community chooses is what it shall be.

I agree with you that taking the current ARB test and then adding some non-uniform flow control (along with removing the layout() qualifiers) to it is the best way to go and I *think* Plamena has offered to do so.

At this point, I can just let for it all to sort out and bow out of the discussion at this point as the series seems to have brought additional issues up that are out-of-scope for what I had in mind with the series (namely something small-simple to expose the extension and not enforcing the limitation of the ARB extension).

-Kevin

-----Original Message-----
From: Rogovin, Kevin 
Sent: Tuesday, August 28, 2018 10:10 PM
To: 'mesa-dev at lists.freedesktop.org' <mesa-dev at lists.freedesktop.org>
Cc: jason at jlekstrand.net
Subject: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

Hi,

> Is that tested?

I have tested it in a simple shader test I made (i.e. not in a dedicated test suite such as dEQP, piglit or something else). The created assembly is identical. The specific example is a shader where I replace calls of beginFragmentShaderOrderingINTEL() with beginInvocationInterlockARB() and the created assembly is the same. Running with INTEL_DEBUG=fs produced identical output except the SSA NIR had the different opcode. AFAIK, the current Mesa implementation of the ARB extension does not in any way shape or form enforce any of "no control flow", "only once and only in main" requirements. Indeed, Mesa happily accepts code even without the endInvocationInterlockARB() call as well.  Personally, I am happy that it is more accepting rather than rejecting the code.

-Kevin