[Mesa-dev] [PATCH v3 4/4] i965/nir: lower TES PatchVerticesIn to a constant when a TCS is present

[Jason Ekstrand]

On Tue, Jan 9, 2018 at 3:21 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:

> When a TCS is present at link time we know the number of vertices in the
> patch and we can lower gl_PatchVerticesIn in the TesEval stage directly
> to a constant. We already have a pass for this that we use in the
> Vulkan pipeline, so we just reuse that.
>
> Notice that the GLSL linker also implements this optimization, which
> we are not removing because other drivers may still depend on it, so
> this should only be useful for OpenGL SPIR-V shaders for now.
> ---
>  src/mesa/drivers/dri/i965/brw_program.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_program.c
> b/src/mesa/drivers/dri/i965/brw_program.c
> index f6c7e4515c..7fae22c620 100644
> --- a/src/mesa/drivers/dri/i965/brw_program.c
> +++ b/src/mesa/drivers/dri/i965/brw_program.c
> @@ -89,15 +89,33 @@ brw_create_nir(struct brw_context *brw,
>     nir_validate_shader(nir);
>
>     /* Lower PatchVerticesIn from system value to uniform. This needs to
> -    * happen before brw_preprocess_nir, since that will lower system
> values.
> +    * happen before brw_preprocess_nir, since that will lower system
> values
> +    * to intrinsics.
> +    *
> +    * We only do this for TES if no TCS is present, since otherwise we
> know
> +    * the number of vertices in the patch at link time and we can lower it
> +    * directly to a constant. We do this in nir_lower_patch_vertices,
> which
> +    * needs to run after brw_nir_preprocess has turned the system values
> +    * into intrinsics.
>      */
> -   if ((stage == MESA_SHADER_TESS_CTRL && brw->screen->devinfo.gen >= 8)
> ||
> -       stage == MESA_SHADER_TESS_EVAL) {
> +   const bool lower_patch_vertices_in_to_uniform =
> +      (stage == MESA_SHADER_TESS_CTRL && brw->screen->devinfo.gen >= 8) ||
> +      (stage == MESA_SHADER_TESS_EVAL &&
> +       !shader_prog->_LinkedShaders[MESA_SHADER_TESS_CTRL]);
> +
> +   if (lower_patch_vertices_in_to_uniform)
>        brw_nir_lower_patch_vertices_in_to_uniform(nir);
> -   }
>
>     nir = brw_preprocess_nir(brw->screen->compiler, nir);
>
> +   if (stage == MESA_SHADER_TESS_EVAL && !lower_patch_vertices_in_to_uniform)
> {
> +      assert(shader_prog->_LinkedShaders[MESA_SHADER_TESS_CTRL]);
> +      struct gl_linked_shader *linked_tcs =
> +         shader_prog->_LinkedShaders[MESA_SHADER_TESS_CTRL];
> +      uint32_t patch_vertices = linked_tcs->Program->info.
> tess.tcs_vertices_out;
> +      nir_lower_tes_patch_vertices(nir, patch_vertices);
> +   }
>

Ugh...  This would be way cleaner if we could put
nir_lower_tes_patch_vertices higher up.  But I went and read the pass and
it's not really set up for that.  We could always tweak the
patch_vertices_to_uniform pass to look for system value intrinsics instead
of uniforms.  But that's a mess for other reasons.  Hopefully one day, we
can clean this up.  Until then, the series is

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

+
>     if (stage == MESA_SHADER_FRAGMENT) {
>        static const struct nir_lower_wpos_ytransform_options wpos_options
> = {
>           .state_tokens = {STATE_INTERNAL, STATE_FB_WPOS_Y_TRANSFORM, 0,
> 0, 0},
> --
> 2.11.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180109/402714c3/attachment.html>