[Mesa-dev] [PATCH v3 4/4] anv: set input_slots_valid on brw_wm_prog_key

[Jason Ekstrand]

On Tue, Jan 10, 2017 at 9:28 AM, Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:

> With shaders using a lot of inputs/outputs, like this (from Gtk+) :
>
> layout(location = 0) in vec2 inPos;
> layout(location = 1) in float inGradientPos;
> layout(location = 2) in flat int inRepeating;
> layout(location = 3) in flat int inStopCount;
> layout(location = 4) in flat vec4 inClipBounds;
> layout(location = 5) in flat vec4 inClipWidths;
> layout(location = 6) in flat ColorStop inStops[8];
>
> layout(location = 0) out vec4 outColor;
>
> we're missing the programming of the input_slots_valid field leading
> to an assert further down the backend code.
>
> Note that we need the shader to be translated from spirv before we can
> get the number of inputs/outputs so we set this in a post function and
> leave the field at 0 for hashing.
>
> v2: Use valid slots of the geometry or vertex stage (Jason)
>
> v3: Use helper to find correct vue map (Jason)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/intel/vulkan/anv_pipeline.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_
> pipeline.c
> index 2c46ef5bf9..b625b35267 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -266,8 +266,6 @@ populate_wm_prog_key(const struct gen_device_info
> *devinfo,
>
>     populate_sampler_prog_key(devinfo, &key->tex);
>
> -   /* TODO: Fill out key->input_slots_valid */
> -
>     /* Vulkan doesn't specify a default */
>     key->high_quality_derivatives = false;
>
> @@ -293,6 +291,19 @@ populate_wm_prog_key(const struct gen_device_info
> *devinfo,
>  }
>
>  static void
> +post_populate_wm_prog_key(const struct anv_pipeline *pipeline,
> +                          const nir_shader *nir,
> +                          struct brw_wm_prog_key *key)
> +{
> +   if (_mesa_bitcount_64(nir->info->inputs_read &
> +                         BRW_FS_VARYING_INPUT_MASK) > 16) {
> +      const struct brw_vue_map *vue_map =
> +         anv_pipeline_get_fs_input_map(pipeline);
> +      key->input_slots_valid = vue_map->slots_valid;
> +   }
> +}
> +
> +static void
>  populate_cs_prog_key(const struct gen_device_info *devinfo,
>                       struct brw_cs_prog_key *key)
>  {
> @@ -616,6 +627,8 @@ anv_pipeline_compile_fs(struct anv_pipeline *pipeline,
>        if (nir == NULL)
>           return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
>
> +      post_populate_wm_prog_key(pipeline, nir, &key);
>

Sorry to make you go into v4 but... we can't do this here.

The whole point of the key is to ensure that we recompile the shader
whenever some important external bit of data changes.  The VUE map from
some other stage is definitely "external" and important. :-)

The first and easiest option would be to get rid of the dependence on NIR
and just set key->input_slots_valid to vue_map->slots_valid all the time
regardless of what NIR says.  This does, of course, mean that we'll end up
doing a lot more recompilation than maybe we wanted to.  Sadly, I'm not
quickly coming up with a good way to do much better.

At some point, I'd like to make it possible to do SPIR-V -> NIR way earlier
on in the compilation process.  At that point, this becomes a lot more
tractable problem.  Short of doing that, however, I think we're probably
stuck with more recompiles than needed.


> +
>        unsigned num_rts = 0;
>        struct anv_pipeline_binding rt_bindings[8];
>        nir_function_impl *impl = nir_shader_get_entrypoint(nir);
> --
> 2.11.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/20170110/792d0b7f/attachment.html>