[Mesa-dev] [Mesa-stable] [PATCH 1/3] radv: Set active_stages the same whether or not shaders were cached

[Alex Smith]

On 1 June 2018 at 22:51, Dylan Baker <dylan at pnwbakers.com> wrote:

> Quoting Samuel Pitoiset (2018-06-01 08:58:42)
> >
> >
> > On 06/01/2018 05:48 PM, Dylan Baker wrote:
> > > Quoting Alex Smith (2018-06-01 07:56:38)
> > >> On 1 June 2018 at 15:48, Dylan Baker <dylan at pnwbakers.com> wrote:
> > >>
> > >>      Quoting Alex Smith (2018-05-31 08:44:18)
> > >>      > With GFX9 merged shaders, active_stages would be set to the
> original
> > >>      > stages specified if shaders were not cached, but to the stages
> still
> > >>      > present after merging if they were.
> > >>      >
> > >>      > Be consistent and use the original stages.
> > >>      >
> > >>      > Signed-off-by: Alex Smith <asmith at feralinteractive.com>
> > >>      > Cc: "18.1" <mesa-stable at lists.freedesktop.org>
> > >>      > ---
> > >>      >  src/amd/vulkan/radv_pipeline.c | 7 ++-----
> > >>      >  1 file changed, 2 insertions(+), 5 deletions(-)
> > >>      >
> > >>      > diff --git a/src/amd/vulkan/radv_pipeline.c
> b/src/amd/vulkan/radv_
> > >>      pipeline.c
> > >>      > index 52734a308a..18dcc43ebe 100644
> > >>      > --- a/src/amd/vulkan/radv_pipeline.c
> > >>      > +++ b/src/amd/vulkan/radv_pipeline.c
> > >>      > @@ -1964,6 +1964,8 @@ void radv_create_shaders(struct
> radv_pipeline
> > >>      *pipeline,
> > >>      >                                 _mesa_sha1_compute(modules[i]
> ->nir->
> > >>      info.name,
> > >>      >
> strlen(modules[i]->
> > >>      nir->info.name),
> > >>      >
> modules[i]->sha1);
> > >>      > +
> > >>      > +                       pipeline->active_stages |=
> > >>      mesa_to_vk_shader_stage(i);
> > >>      >                 }
> > >>      >         }
> > >>      >
> > >>      > @@ -1979,10 +1981,6 @@ void radv_create_shaders(struct
> radv_pipeline
> > >>      *pipeline,
> > >>      >
> > >>      >         if (radv_create_shader_variants_
> from_pipeline_cache(device,
> > >>      cache, hash, pipeline->shaders) &&
> > >>      >             (!modules[MESA_SHADER_GEOMETRY] ||
> pipeline->gs_copy_shader))
> > >>      {
> > >>      > -               for (unsigned i = 0; i < MESA_SHADER_STAGES;
> ++i) {
> > >>      > -                       if (pipeline->shaders[i])
> > >>      > -                               pipeline->active_stages |=
> > >>      mesa_to_vk_shader_stage(i);
> > >>      > -               }
> > >>      >                 return;
> > >>      >         }
> > >>      >
> > >>      > @@ -2015,7 +2013,6 @@ void radv_create_shaders(struct
> radv_pipeline
> > >>      *pipeline,
> > >>      >                                                     stage ?
> stage->pName
> > >>      : "main", i,
> > >>      >                                                     stage ?
> stage->
> > >>      pSpecializationInfo : NULL,
> > >>      >                                                     flags);
> > >>      > -               pipeline->active_stages |=
> mesa_to_vk_shader_stage(i);
> > >>      >
> > >>      >                 /* We don't want to alter meta shaders IR
> directly so
> > >>      clone it
> > >>      >                  * first.
> > >>      > --
> > >>      > 2.14.3
> > >>      >
> > >>      > _______________________________________________
> > >>      > mesa-stable mailing list
> > >>      > mesa-stable at lists.freedesktop.org
> > >>      > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> > >>
> > >>      Hi Alex,
> > >>
> > >>      This doesn't apply cleanly to the 18.1 tree with the following
> collision:
> > >>
> > >>      ++<<<<<<< HEAD
> > >>
> > >>       +                                                  stage ?
> stage->
> > >>      pSpecializationInfo : NULL);
> > >>       +              pipeline->active_stages |=
> mesa_to_vk_shader_stage(i);
> > >>
> > >>      ++=======
> > >>
> > >>      +                                                   stage ?
> stage->
> > >>      pSpecializationInfo : NULL,
> > >>      +                                                   flags);
> > >>
> > >>      ++>>>>>>> 0fa51bfdbe5... radv: Set active_stages the same
> whether or not
> > >>      shaders were cached
> > >>
> > >>      I can remove the flags field (which doesn't exist in 18.1)
> before merging
> > >>      if you
> > >>      think that's the right thing to do. Does that seem reasonable?
> > >>
> > >>
> > >> Yes, that's correct. Should just be removing the
> pipeline->active_stages line
> > >> at that point.
> > >>
> > >> Thanks,
> > >> Alex
> > >
> > > Thanks! This is in the tree for 18.1.2.
> >
> > Apparently, this series regresses some CTS on my side, Bas will
> > investigate, would be nice to not release 18.1.2 without this fixed. :)
> >
>
> Let me know what you all want to do, 18.1.1 happened today, and 18.1.2 is
> planned for release on the 15th, so you have some time to figure it out.
>

This commit fixes the issue:
https://cgit.freedesktop.org/mesa/mesa/commit/?id=2835b6baf446d0ff3b3df6eefc57b248a505af36

Thanks,
Alex


>
> Dylan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180604/ab51c173/attachment-0001.html>