<div dir="ltr">Thanks for fixing this!<br><div><br>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 16, 2017 at 4:56 AM, Iago Toral Quiroga <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The same we do in the OpenGL driver (comment copied from there).<br>
<br>
This is required to ensure that we execute the fragment shader stage when<br>
side-effects (such as image or ssbo stores) are present but there are no<br>
color writes.<br>
<br>
I found this while writing a test to check rendering to a framebuffer<br>
without attachments where the fragment shader does not produce any<br>
color outputs but writes to an image via imageStore(). Without this patch<br>
the fragment shader does not execute and the image is not written,<br>
which is not correct.<br>
---<br>
 src/intel/vulkan/genX_<wbr>pipeline.c | 51 ++++++++++++++++++++++++++++++<wbr>++++++++++<br>
 1 file changed, 51 insertions(+)<br>
<br>
diff --git a/src/intel/vulkan/genX_<wbr>pipeline.c b/src/intel/vulkan/genX_<wbr>pipeline.c<br>
index 90968b4..6a927d5 100644<br>
--- a/src/intel/vulkan/genX_<wbr>pipeline.c<br>
+++ b/src/intel/vulkan/genX_<wbr>pipeline.c<br>
@@ -1248,6 +1248,26 @@ emit_3dstate_ps(struct anv_pipeline *pipeline,<br>
    }<br>
 }<br>
<br>
+static inline bool<br>
+has_color_buffer_write_<wbr>enabled(const struct anv_pipeline *pipeline)<br>
+{<br>
+   const struct anv_shader_bin *shader_bin =<br>
+      pipeline->shaders[MESA_SHADER_<wbr>FRAGMENT];<br>
+   if (!shader_bin)<br>
+      return false;<br>
+<br>
+   const struct anv_pipeline_bind_map *bind_map = &shader_bin->bind_map;<br>
+   for (int i = 0; i < bind_map->surface_count; i++) {<br>
+      if (bind_map->surface_to_<wbr>descriptor[i].set !=<br>
+          ANV_DESCRIPTOR_SET_COLOR_<wbr>ATTACHMENTS)<br>
+         continue;<br>
+      if (bind_map->surface_to_<wbr>descriptor[i].index != UINT8_MAX)<br>
+         return true;<br>
+   }<br>
+<br>
+   return false;<br>
+}<br>
+<br>
 #if GEN_GEN >= 8<br>
 static void<br>
 emit_3dstate_ps_extra(struct anv_pipeline *pipeline,<br>
@@ -1278,6 +1298,37 @@ emit_3dstate_ps_extra(struct anv_pipeline *pipeline,<br>
       ps.PixelShaderKillsPixel         = subpass->has_ds_self_dep ||<br>
                                          wm_prog_data->uses_kill;<br>
<br>
+      /* The stricter cross-primitive coherency guarantees that the hardware<br>
+       * gives us with the "Accesses UAV" bit set for at least one shader stage<br>
+       * and the "UAV coherency required" bit set on the 3DPRIMITIVE command are<br>
+       * redundant within the current image, atomic counter and SSBO GL APIs,<br>
+       * which all have very loose ordering and coherency requirements and<br>
+       * generally rely on the application to insert explicit barriers when a<br>
+       * shader invocation is expected to see the memory writes performed by the<br>
+       * invocations of some previous primitive.  Regardless of the value of<br>
+       * "UAV coherency required", the "Accesses UAV" bits will implicitly cause<br>
+       * an in most cases useless DC flush when the lowermost stage with the bit<br>
+       * set finishes execution.<br>
+       *<br>
+       * It would be nice to disable it, but in some cases we can't because on<br>
+       * Gen8+ it also has an influence on rasterization via the PS UAV-only<br>
+       * signal (which could be set independently from the coherency mechanism<br>
+       * in the 3DSTATE_WM command on Gen7), and because in some cases it will<br>
+       * determine whether the hardware skips execution of the fragment shader<br>
+       * or not via the ThreadDispatchEnable signal.  However if we know that<br>
+       * GEN8_PS_BLEND_HAS_WRITEABLE_RT is going to be set and<br>
+       * GEN8_PSX_PIXEL_SHADER_NO_RT_<wbr>WRITE is not set it shouldn't make any<br>
+       * difference so we may just disable it here.<br>
+       *<br>
+       * Gen8 hardware tries to compute ThreadDispatchEnable for us but doesn't<br>
+       * take into account KillPixels when no depth or stencil writes are<br>
+       * enabled. In order for occlusion queries to work correctly with no<br>
+       * attachments, we need to force-enable here.<br>
+       */<br>
+      if ((wm_prog_data->has_side_<wbr>effects || wm_prog_data->uses_kill) &&<br>
+          !has_color_buffer_write_<wbr>enabled(pipeline))<br>
+         ps.PixelShaderHasUAV = true;<br>
+<br>
 #if GEN_GEN >= 9<br>
       ps.PixelShaderPullsBary    = wm_prog_data->pulls_bary;<br>
       ps.InputCoverageMaskState  = wm_prog_data->uses_sample_mask ?<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.7.4<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div></div></div>