<div><div>Hi!</div><div> </div><div>I'm currently preparing the implementation of fragment shader interlock in the Mesa RADV driver for merging (merge request !22250), and one Piglit test fails on Zink on RADV with my implementation.</div><div> </div><div>The test is spec/arb_fragment_shader_interlock/image-load-store.</div><div> </div><div>I noticed that it used layout(pixel_interlock_ordered) with sample shading. This is a combination that is only available in OpenGL and Vulkan, but not in Direct3D, Metal and GL_INTEL_fragment_shader_ordering (which force sample interlock if sample shading is used), and given that AMD hasn't exposed the KHR/EXT fragment shader interlock in their official OpenGL and Vulkan drivers so far (unlike in Direct3D and Metal), I was starting to think that they were unable to support the full requirements of pixel_interlock at all.</div><div> </div><div>However, they actually do support pixel_interlock with sample shading. I was able to verify that by modifying an existing example (`Triang3l/vk_order_independent_transparency` on GitHub, `radv_sample_shading_test` branch) of fragment shader interlock usage with per-pixel data to use MSAA and sample shading, but only accessing that data once per pixel for each primitive, by wrapping all pixel-local data accesses in:</div><div> </div><div>`if (gl_SampleID == findLSB(gl_SampleMaskIn[0])) { /* ... */ }`</div><div> </div><div>so that for each fragment belonging to one primitive, the pixel-local data is accessed from just one sample-rate invocation. With my change, as expected with pixel_interlock_ordered, at common edges of adjacent primitives, which cover the same pixels, but don't have common covered samples, I got pixel values that were stable between frames, plus a huge speed loss. And when I changed the mode to sample_interlock_ordered (in AMD hardware terms, setting POPS_OVERLAP_NUM_SAMPLES to MSAA_EXPOSED_SAMPLES rather than 1x), the edges became unstable and noisy, and everything ran much faster. So, AMD GPUs actually support pixel_interlock, and if you have POPS_OVERLAP_NUM_SAMPLES = 1x, you still get interlock between primitives with no common samples as pixel_interlock requires, unlike in Direct3D, even if sample shading is used.</div><div> </div><div>So it turns out that the Piglit test itself apparently requires the guarantees that neither the OpenGL (explicitly) nor the Vulkan specification provides, and that AMD hardware rightfully doesn't implement, though Intel (who created that Piglit test) actually happens to offer them due to the details of how their GPUs seem to work.</div><div> </div><div>Specifically, the following two parts are problematic:</div><div> </div><div>ivec3 current_sample_coord = ivec3(gl_FragCoord.x, gl_FragCoord.y, gl_SampleID);</div><div>// ...</div><div>imageStore(img_output, current_sample_coord, result);</div><div> </div><div>and:</div><div> </div><div>for (i = 0; i < sample_rate; i++) {<!-- --></div><div>  if (i != gl_SampleID) {<!-- --></div><div>    ivec3 sample_coord = ivec3(gl_FragCoord.x, gl_FragCoord.y, i);</div><div>    vec4 sample_color = imageLoad(img_output, sample_coord);</div><div>    // ...</div><div>  }</div><div>}</div><div> </div><div>In this test, each invocation (corresponding to one MSAA sample) writes to a location for its sample, and reads the data for all other samples in the pixel.</div><div> </div><div>However, most of the time (if a primitive covers more than 1 sample of a pixel, thus everywhere in the middle of a primitive) this results in invocations accessing data (for other samples in the pixel) written by invocations belonging to other samples in __the same primitive__.</div><div> </div><div>According to the specification of GL_ARB_fragment_shader_interlock, no synchronization is guaranteed in this case:</div><div> </div><div>    (1) When using multisampling, the OpenGL specification permits</div><div>        multiple fragment shader invocations to be generated for a single</div><div>        fragment.  For example, per-sample shading using the "sample"</div><div>        auxiliary storage qualifier or the MinSampleShading() OpenGL API command</div><div>        can be used to force per-sample shading.  What execution ordering</div><div>        guarantees are provided between fragment shader invocations generated</div><div>        from the same fragment?</div><div> </div><div>      RESOLVED:  We don't provide any ordering guarantees in this extension.</div><div>      This implies that when using multisampling, there is no guarantee that</div><div>      two fragment shader invocations for the same fragment won't be executing</div><div>      their critical sections concurrently.  This could cause problems for</div><div>      algorithms sharing data structures between all the samples of a pixel</div><div>      unless accesses to these data structures are performed atomically.</div><div> </div><div>      When using per-sample shading, the interlock we provide *does* guarantee</div><div>      that no two invocations corresponding to the same sample execute the</div><div>      critical section concurrently.  If a separate set of data structures is</div><div>      provided for each sample, no conflicts should occur within the critical</div><div>      section.</div><div> </div><div>As the last paragraph of the quote says, "the interlock we provide does guarantee that no two invocations corresponding to __the same sample__ execute the critical section concurrently". But in this test, the same memory location is accessed by invocations corresponding to __different samples__. And thus, the regular rule for invocations generated from the same fragment applies: "there is no guarantee that two fragment shader invocations for the same fragment won't be executing their critical sections concurrently".</div><div> </div><div>In Vulkan, although its specification does not apply to OpenGL but still can provide additional background information, fragment shader interlock synchronization scope is defined in terms of rasterization order:</div><div> </div><div>> If the PixelInterlockOrderedEXT execution mode is specified, any interlocked operations in a fragment shader must happen before interlocked operations in fragment shader invocations that execute later in rasterization order and cover at least one sample in the same pixel, and must happen after interlocked operations in a fragment shader that executes earlier in rasterization order and cover at least one sample in the same pixel.</div><div> </div><div>> If the PixelInterlockUnorderedEXT execution mode is specified, any interlocked operations in a fragment shader must happen before or after interlocked operations in fragment shader invocations that execute earlier or later in rasterization order and cover at least one sample in the same pixel.</div><div> </div><div>However, no ordering guarantees are specified for the case when shader invocations execute __neither earlier nor later__ than each other in rasterization order.</div><div> </div><div>It appears that Intel actually provides the guarantee for invocations for the same fragment, but it's not something that OpenGL requires, and not all vendors are implementing, with AMD being one of those who don't. And this issue apparently hasn't manifested itself until now because currently Intel is the only vendor with fragment shader interlock implemented in the Mesa upstream.</div><div> </div><div>The Vulkan CTS contains some tests that seem to include the combination of pixel_interlock and sample shading in VK-GL-CTS/external/vulkancts/modules/vulkan/fragment_shader_interlock, and both Intel and AMD GPUs pass them successfully. I think the current broken test may be replaced with a port of those tests, that cover many additional cases like `discard` in various locations relative to the critical section.</div><div> </div><div>Thank you!</div><div>-- Triang3l</div></div>