<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>