[Mesa-dev] [PATCH v6 2/2] anv: enable VK_EXT_shader_stencil_export

[Jason Ekstrand]

On Fri, May 11, 2018 at 12:15 PM, Caio Marcelo de Oliveira Filho <
caio.oliveira at intel.com> wrote:

> From: Gustavo Lima Chaves <gustavo.lima.chaves at intel.com>
>
> v2:
> An attempt to support SpvExecutionModeStencilRefReplacingEXT's behavior
> also follows, with the interpretation to said mode being we prevent
> writes to the built-in FragStencilRefEXT variable when the execution
> mode isn't set.
>
> v3:
> A more cautious reading of 1db44252d01bf7539452ccc2b5210c74b8dcd573 led
> me to a missing change that would stop (what I later discovered were)
> GPU hangs on the CTS test written to exercize this.
>
> v4:
> Turn FragStencilRefEXT decoration usage without StencilRefReplacingEXT
> mode into a warning, instead of trying to make the variable read-only.
> If we are to follow the originating extension on GL, the built-in
> variable in question should never be readable anyway.
> ---
>
> This is the original patch from Gustavo Lima Chaves (v5 was another
> rebase), with some trivial changes to rebase.
>
> I'm updating the test on Vulkan CTS to make sure we have some coverage
> of this feature; so this should only land once that gets accepted, but
> is ready for review.
>
>  src/compiler/shader_info.h         | 2 ++
>  src/compiler/spirv/spirv_to_nir.c  | 4 ++++
>  src/compiler/spirv/vtn_variables.c | 5 +++++
>  src/intel/vulkan/anv_extensions.py | 1 +
>  src/intel/vulkan/anv_pipeline.c    | 1 +
>  src/intel/vulkan/genX_pipeline.c   | 1 +
>  6 files changed, 14 insertions(+)
>
> diff --git a/src/compiler/shader_info.h b/src/compiler/shader_info.h
> index 81f844d36ae..e9141261e8b 100644
> --- a/src/compiler/shader_info.h
> +++ b/src/compiler/shader_info.h
> @@ -180,6 +180,8 @@ typedef struct shader_info {
>
>           bool pixel_center_integer;
>
> +         bool outputs_stencil;
>

This isn't used by anything outside of spirv_to_nir so I don't think it
makes sense in shader_info.  If user wants to know if the output is
written, they can look at outputs_written.  If you want to have the warning
below (which seems good to me), maybe a boolean in vtn_builder instead?


> +
>           /** gl_FragDepth layout for ARB_conservative_depth. */
>           enum gl_frag_depth_layout depth_layout;
>        } fs;
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 6c0551603ea..81c5e890071 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -3577,6 +3577,10 @@ vtn_handle_execution_mode(struct vtn_builder *b,
> struct vtn_value *entry_point,
>     case SpvExecutionModeContractionOff:
>        break; /* OpenCL */
>
> +   case SpvExecutionModeStencilRefReplacingEXT:
> +      b->shader->info.fs.outputs_stencil = true;
> +      break;
> +
>     default:
>        vtn_fail("Unhandled execution mode");
>     }
> diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_
> variables.c
> index 53bee1b9288..0a3108bdc43 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1429,6 +1429,11 @@ apply_var_decoration(struct vtn_builder *b,
> nir_variable *nir_var,
>        case SpvBuiltInSamplePosition:
>           nir_var->data.origin_upper_left = b->origin_upper_left;
>           break;
> +      case SpvBuiltInFragStencilRefEXT:
> +         if (!b->shader->info.fs.outputs_stencil)
> +            vtn_warn("The StencilRefReplacingEXT mode should be declared
> when"
> +                     " the decoration FragStencilRefEXT is used on a
> variable");
> +         break;
>

This stuff should go in the previous patch.


>        default:
>           break;
>        }
> diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_
> extensions.py
> index b5bee0881ce..8160864685f 100644
> --- a/src/intel/vulkan/anv_extensions.py
> +++ b/src/intel/vulkan/anv_extensions.py
> @@ -112,6 +112,7 @@ EXTENSIONS = [
>      Extension('VK_EXT_global_priority',                   1,
>                'device->has_context_priority'),
>      Extension('VK_EXT_shader_viewport_index_layer',       1, True),
> +    Extension('VK_EXT_shader_stencil_export',             1,
> 'device->info.gen >= 9'),
>  ]
>
>  class VkVersion:
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_
> pipeline.c
> index 8f30136b100..c37b9b96e11 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -152,6 +152,7 @@ anv_shader_compile_to_nir(struct anv_pipeline
> *pipeline,
>           .subgroup_quad = true,
>           .subgroup_shuffle = true,
>           .subgroup_vote = true,
> +         .stencil_export = true,
>        },
>     };
>
> diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_
> pipeline.c
> index 6016d257584..462c59451cc 100644
> --- a/src/intel/vulkan/genX_pipeline.c
> +++ b/src/intel/vulkan/genX_pipeline.c
> @@ -1600,6 +1600,7 @@ emit_3dstate_ps_extra(struct anv_pipeline *pipeline,
>           ps.PixelShaderHasUAV = true;
>
>  #if GEN_GEN >= 9
> +      ps.PixelShaderComputesStencil = wm_prog_data->computed_stencil;
>        ps.PixelShaderPullsBary    = wm_prog_data->pulls_bary;
>        ps.InputCoverageMaskState  = wm_prog_data->uses_sample_mask ?
>                                     ICMS_INNER_CONSERVATIVE : ICMS_NONE;
> --
> 2.17.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180511/e20954ca/attachment.html>