[Mesa-dev] [PATCH v2] anv: set input_slots_valid on brw_wm_prog_key

[Jason Ekstrand]

On Tue, Jan 10, 2017 at 8:59 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> On Tue, Jan 10, 2017 at 11:50 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)
> >
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > Cc: Jason Ekstrand <jason at jlekstrand.net>
> > Cc: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/intel/vulkan/anv_pipeline.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_
> pipeline.c
> > index 2c46ef5bf9..e51149cf37 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,23 @@ 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) {
> > +      struct anv_shader_bin * const *shaders = pipeline->shaders;
> > +      const struct brw_vue_prog_data *vue_prog_data =
> > +         shaders[MESA_SHADER_GEOMETRY] ?
> > +         (struct brw_vue_prog_data *) shaders[MESA_SHADER_GEOMETRY]->prog_data
> :
>
> As you're about to get tess support, maybe just take that into account
> here? It should fall back as GP -> TEP -> VP. (Presumably the logic
> you're copying from has this as well.)
>

Agreed.  I think what we actually want is to add a
anv_pipeline_get_fs_input_map() helper that returns the VUE map and takes
these things into account.  We have to do the same thing in
emit_3dstate_sbe in genX_pipeline.h so it may be worth the deduplication.
Could be

if (pipeline_has_stage(pipeline, MESA_SHADER_GEOMETRY))
   return &anv_get_gs_prog_data(pipeline)->base.vue_map;
else
   return &anv_get_vs_prog_data(pipeline)->base.vue_map;

with the tessellation one in there too.


> > +         (struct brw_vue_prog_data *) shaders[MESA_SHADER_VERTEX]->
> prog_data;
> > +
> > +      key->input_slots_valid = vue_prog_data->vue_map.slots_valid;
> > +   }
> > +}
> > +
> > +static void
> >  populate_cs_prog_key(const struct gen_device_info *devinfo,
> >                       struct brw_cs_prog_key *key)
> >  {
> > @@ -616,6 +631,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);
> > +
> >        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
> _______________________________________________
> 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/2e8635f2/attachment.html>