<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 10, 2017 at 2:55 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Feb 10, 2017 at 11:02:19AM -0800, Jason Ekstrand wrote: > It's a bit hard to measure because it almost gets lost in the noise, > but this seemed to help Dota 2 by a percent or two on my Broadwell > GT3e desktop. > > Reviewed-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a>> > --- >Â src/intel/vulkan/genX_pipeline.c | 171 +++++++++++++++++++++++++++++++-------- >Â 1 file changed, 137 insertions(+), 34 deletions(-) > > diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c > index 71b9e30..d2af8b9 100644 > --- a/src/intel/vulkan/genX_pipeline.c > +++ b/src/intel/vulkan/genX_pipeline.c > @@ -638,6 +638,133 @@ static const uint32_t vk_to_gen_stencil_op[] = { >Â Â Â [VK_STENCIL_OP_DECREMENT_AND_WRAP]Â Â Â Â Â Â = STENCILOP_DECR, >Â }; > > +static bool > +may_write_stencil_face(const VkStencilOpState *face, > +Â Â Â Â Â Â Â Â Â Â Â Â VkCompareOp depthCompareOp) > +{ This function triggers some false negatives. One example is face->compareOp == VK_COMPARE_OP_GREATER && face->depthFailOp == VK_STENCIL_OP_KEEP && face->passOp == VK_STENCIL_OP_KEEP && face->failOp == VK_STENCIL_OP_INVERT && depthCompareOp == don't care; This function returns false even though a stencil write occurs if the stencil test results in a fail. The possible input combinations that affect writing to stencil can be divided up into 10 sets. sÂ = stencil compare op dÂ = depth compare op df = depth fail op sf = stencil fail op sp = stencil pass op Â Â Â Â Â Â Â Â Â Â s.AlwaysÂ Â Â Â Â Â Â |Â Â Â Â Â !s.Always Â Â Â Â Â Â Â d.NeverÂ |Â Â Â !d.NeverÂ Â |Â Â s.NeverÂ Â | Â Â Â Â Â Â Â Â Â Â Â Â | d.Always |Â Â Â |Â Â Â Â Â Â Â | !keepÂ Â Â Â Â Â Â Â Â |Â Â Â Â Â |Â Â Â |Â Â Â Â Â Â Â | Â Â Â Â Â Â Â Â Â Â Â Â |Â Â Â Â Â |Â Â Â |Â Â Â Â Â Â Â | Â Â Â Â Â Â ------------|----------|------|--------------|-------------- Â Â Â Â Â Â Â Â Â Â Â Â |Â Â Â Â Â |Â Â Â |Â Â Â Â Â Â Â | keepÂ Â Â Â Â df.KeepÂ Â | sp.KeepÂ |Â Â Â |Â Â sf.KeepÂ Â | Â Â Â Â Â Â Â Â Â Â Â Â |Â Â Â Â Â |Â Â Â |Â Â Â Â Â Â Â | Â Â Â Â Â Â Â Â Â Â Â Â |Â Â Â Â Â |Â Â Â |Â Â Â Â Â Â Â | We know that stencil won't be written if the function inputs land us in 3 of these sets (denoted by the *.Keep's above). I think this function would be easier to understand if it determined if we fell in one of the 3 instead of the 7. How about something like this: static bool wont_write_stencil_face(const VkStencilOpState *face, Â Â Â Â Â Â Â Â Â Â Â Â VkCompareOp depthCompareOp) { Â Â if (face->compareOp == VK_COMPARE_OP_NEVER && Â Â Â Â face->failOp == VK_STENCIL_OP_KEEP) Â Â Â Â return true; Â Â if (face->compareOp == VK_COMPARE_OP_ALWAYS) { Â Â Â Â if (depthCompareOp == VK_COMPARE_OP_NEVER && Â Â Â Â Â Â face->depthFailOp == VK_STENCIL_OP_KEEP) Â Â Â Â Â Â return true; Â Â Â Â if (depthCompareOp == VK_COMPARE_OP_ALWAYS && Â Â Â Â Â Â face->passOp == VK_STENCIL_OP_KEEP) Â Â Â Â Â Â return true; Â Â } </blockquote><div> </div><div>This covers a lot fewer cases.Â For instance, if all of the stencil ops are VK_STENCIL_OP_KEEP but compareOp == VK_COMPARE_OP_GREATER, your function will claim that it writes stencil even though it clearly doesn't. </div><div>Â </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Â Â return false; } -Nanley <div class="HOEnZb"><div class="h5"> > +Â Â /* If compareOp is ALWAYS then the stencil test will never fail and we can > +Â Â * ignore failOp.Â If it's not ALWAYS and failOp is not KEEP, we may write > +Â Â * stencil. > +Â Â */ > +Â Â if (face->compareOp != VK_COMPARE_OP_ALWAYS && > +Â Â Â Â face->failOp != VK_STENCIL_OP_KEEP) > +Â Â Â return true; > + > + > +Â Â /* If the compareOp is NEVER then the stencil test will never pass and the > +Â Â * passOp and depthFailOp don't matter.Â If compareOp isn't NEVER, then we > +Â Â * need to take them into account. > +Â Â */ > +Â Â if (face->compareOp != VK_COMPARE_OP_NEVER) { > +Â Â Â /* If depthOp is NEVER then passOp doesn't matter. */ > +Â Â Â if (depthCompareOp != VK_COMPARE_OP_NEVER && > +Â Â Â Â Â face->passOp != VK_STENCIL_OP_KEEP) > +Â Â Â Â Â return true; > + > +Â Â Â /* If depthOp is ALWAYS then depthFailOp doesn't matter. */ > +Â Â Â if (depthCompareOp != VK_COMPARE_OP_ALWAYS && > +Â Â Â Â Â face->depthFailOp != VK_STENCIL_OP_KEEP) > +Â Â Â Â Â return true; > +Â Â } > + > +Â Â return false; > +} > + > +/* Intel hardware is fairly sensitive to whether or not depth/stencil writes > + * are enabled.Â In the presence of discards, it's fairly easy to get into the > + * non-promoted case which means a fairly big performance hit.Â >From the Iron > + * Lake PRM, Vol 2, pt. 1, section 8.4.3.2, "Early Depth Test Cases": > + * > + *Â Â "Non-promoted depth (N) is active whenever the depth test can be done > + *Â Â early but it cannot determine whether or not to write source depth to > + *Â Â the depth buffer, therefore the depth write must be performed post pixel > + *Â Â shader. This includes cases where the pixel shader can kill pixels, > + *Â Â including via sampler chroma key, as well as cases where the alpha test > + *Â Â function is enabled, which kills pixels based on a programmable alpha > + *Â Â test. In this case, even if the depth test fails, the pixel cannot be > + *Â Â killed if a stencil write is indicated. Whether or not the stencil write > + *Â Â happens depends on whether or not the pixel is killed later. In these > + *Â Â cases if stencil test fails and stencil writes are off, the pixels can > + *Â Â also be killed early. If stencil writes are enabled, the pixels must be > + *Â Â treated as Computed depth (described above)." > + * > + * The same thing as mentioned in the stencil case can happen in the depth > + * case as well if it thinks it writes depth but, thanks to the depth test > + * being GL_EQUAL, the write doesn't actually matter.Â A little extra work > + * up-front to try and disable depth and stencil writes can make a big > + * difference. > + * > + * Unfortunately, the way depth and stencil testing is specified, there are > + * many case where, regardless of depth/stencil writes being enabled, nothing > + * actually gets written due to some other bit of state being set.Â This > + * function attempts to "sanitize" the depth stencil state and disable writes > + * and sometimes even testing whenever possible. > + */ > +static void > +sanitize_ds_state(VkPipelineDepthStencilStateCreateInfo *state, > +Â Â Â Â Â Â Â Â Â bool *stencilWriteEnable, > +Â Â Â Â Â Â Â Â Â VkImageAspectFlags ds_aspects) > +{ > +Â Â *stencilWriteEnable = state->stencilTestEnable; > + > +Â Â /* If the depth test is disabled, we won't be writing anything. */ > +Â Â if (!state->depthTestEnable) > +Â Â Â state->depthWriteEnable = false; > + > +Â Â /* The Vulkan spec requires that if either depth or stencil is not present, > +Â Â * the pipeline is to act as if the test silently passes. > +Â Â */ > +Â Â if (!(ds_aspects & VK_IMAGE_ASPECT_DEPTH_BIT)) { > +Â Â Â state->depthWriteEnable = false; > +Â Â Â state->depthCompareOp = VK_COMPARE_OP_ALWAYS; > +Â Â } > + > +Â Â if (!(ds_aspects & VK_IMAGE_ASPECT_STENCIL_BIT)) { > +Â Â Â *stencilWriteEnable = false; > +Â Â Â state->front.compareOp = VK_COMPARE_OP_ALWAYS; > +Â Â Â state->back.compareOp = VK_COMPARE_OP_ALWAYS; > +Â Â } > + > +Â Â /* If the stencil test is enabled and always fails, then we will never get > +Â Â * to the depth test so we can just disable the depth test entirely. > +Â Â */ > +Â Â if (state->stencilTestEnable && > +Â Â Â Â state->front.compareOp == VK_COMPARE_OP_NEVER && > +Â Â Â Â state->back.compareOp == VK_COMPARE_OP_NEVER) { > +Â Â Â state->depthTestEnable = false; > +Â Â Â state->depthWriteEnable = false; > +Â Â } > + > +Â Â /* If depthCompareOp is EQUAL then the value we would be writing to the > +Â Â * depth buffer is the same as the value that's already there so there's no > +Â Â * point in writing it. > +Â Â */ > +Â Â if (state->depthCompareOp == VK_COMPARE_OP_EQUAL) > +Â Â Â state->depthWriteEnable = false; > + > +Â Â /* If the stencil ops are such that we don't actually ever modify the > +Â Â * stencil buffer, we should disable writes. > +Â Â */ > +Â Â if (!may_write_stencil_face(&state->front, state->depthCompareOp) && > +Â Â Â Â !may_write_stencil_face(&state->back, state->depthCompareOp)) > +Â Â Â *stencilWriteEnable = false; > + > +Â Â /* If the depth test always passes and we never write out depth, that's the > +Â Â * same as if the depth test is disabled entirely. > +Â Â */ > +Â Â if (state->depthCompareOp == VK_COMPARE_OP_ALWAYS && > +Â Â Â Â !state->depthWriteEnable) > +Â Â Â state->depthTestEnable = false; > + > +Â Â /* If the stencil test always passes and we never write out stencil, that's > +Â Â * the same as if the stencil test is disabled entirely. > +Â Â */ > +Â Â if (state->front.compareOp == VK_COMPARE_OP_ALWAYS && > +Â Â Â Â state->back.compareOp == VK_COMPARE_OP_ALWAYS && > +Â Â Â Â !*stencilWriteEnable) > +Â Â Â state->stencilTestEnable = false; > +} > + >Â static void >Â emit_ds_state(struct anv_pipeline *pipeline, >Â Â Â Â Â Â Â Â const VkPipelineDepthStencilStateCreateInfo *pCreateInfo, > @@ -663,12 +790,20 @@ emit_ds_state(struct anv_pipeline *pipeline, >Â Â Â Â return; >Â Â Â } > > +Â Â VkImageAspectFlags ds_aspects = 0; > +Â Â if (subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) { > +Â Â Â VkFormat depth_stencil_format = > +Â Â Â Â Â pass->attachments[subpass->depth_stencil_attachment].format; > +Â Â Â ds_aspects = vk_format_aspects(depth_stencil_format); > +Â Â } > + >Â Â Â VkPipelineDepthStencilStateCreateInfo info = *pCreateInfo; > +Â Â sanitize_ds_state(&info, &pipeline->writes_stencil, ds_aspects); > +Â Â pipeline->writes_depth = info.depthWriteEnable; > +Â Â pipeline->depth_test_enable = info.depthTestEnable; > >Â Â Â /* VkBool32 depthBoundsTestEnable; // optional (depth_bounds_test) */ > > -Â Â pipeline->writes_stencil = info.stencilTestEnable; > - >Â #if GEN_GEN <= 7 >Â Â Â struct GENX(DEPTH_STENCIL_STATE) depth_stencil = { >Â #else > @@ -690,38 +825,6 @@ emit_ds_state(struct anv_pipeline *pipeline, >Â Â Â Â .BackfaceStencilTestFunction = vk_to_gen_compare_op[info.back.compareOp], >Â Â Â }; > > -Â Â VkImageAspectFlags aspects = 0; > -Â Â if (subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) { > -Â Â Â VkFormat depth_stencil_format = > -Â Â Â Â Â pass->attachments[subpass->depth_stencil_attachment].format; > -Â Â Â aspects = vk_format_aspects(depth_stencil_format); > -Â Â } > - > -Â Â /* The Vulkan spec requires that if either depth or stencil is not present, > -Â Â * the pipeline is to act as if the test silently passes. > -Â Â */ > -Â Â if (!(aspects & VK_IMAGE_ASPECT_DEPTH_BIT)) { > -Â Â Â depth_stencil.DepthBufferWriteEnable = false; > -Â Â Â depth_stencil.DepthTestFunction = PREFILTEROPALWAYS; > -Â Â } > - > -Â Â if (!(aspects & VK_IMAGE_ASPECT_STENCIL_BIT)) { > -Â Â Â pipeline->writes_stencil = false; > -Â Â Â depth_stencil.StencilTestFunction = PREFILTEROPALWAYS; > -Â Â Â depth_stencil.BackfaceStencilTestFunction = PREFILTEROPALWAYS; > -Â Â } > - > -Â Â /* From the Broadwell PRM: > -Â Â * > -Â Â *Â Â "If Depth_Test_Enable = 1 AND Depth_Test_func = EQUAL, the > -Â Â *Â Â Depth_Write_Enable must be set to 0." > -Â Â */ > -Â Â if (info.depthTestEnable && info.depthCompareOp == VK_COMPARE_OP_EQUAL) > -Â Â Â depth_stencil.DepthBufferWriteEnable = false; > - > -Â Â pipeline->writes_depth = depth_stencil.DepthBufferWriteEnable; > -Â Â pipeline->depth_test_enable = depth_stencil.DepthTestEnable; > - >Â #if GEN_GEN <= 7 >Â Â Â GENX(DEPTH_STENCIL_STATE_pack)(NULL, depth_stencil_dw, &depth_stencil); >Â #else > -- > 2.5.0.400.gff86faf > </div></div><div class="HOEnZb"><div class="h5">> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>