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 <lionel.g.landwer...@intel.com>
> ---
> 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;
}
return false;
}
-Nanley
> + /* 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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
