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

[Samuel Pitoiset]

On 06/01/2018 06:28 PM, Alex Smith wrote:
> On 1 June 2018 at 16:58, Samuel Pitoiset <samuel.pitoiset at gmail.com 
> <mailto:samuel.pitoiset at gmail.com>> wrote:
> 
> 
> 
>     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
>             <mailto: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
>             <mailto:asmith at feralinteractive.com>>
>                   > Cc: "18.1" <mesa-stable at lists.freedesktop.org
>             <mailto: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 <http://info.name>,
>                   >                                                   
>             strlen(modules[i]->
>                   nir->info.name <http://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
>             <mailto:mesa-stable at lists.freedesktop.org>
>                   >
>             https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>             <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. :)
> 
> 
> What tests are failing?

deqp-vk/pipeline/push_constant/graphics_pipeline/count_3_shaders_vert_geom_frag: 
fail
deqp-vk/binding_model/shader_access/primary_cmd_buf/storage_buffer_dynamic/geometry/*

(polaris10)

> 
> 
> 
>         Dylan
> 
> 
> 
>         _______________________________________________
>         mesa-dev mailing list
>         mesa-dev at lists.freedesktop.org
>         <mailto:mesa-dev at lists.freedesktop.org>
>         https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>         <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> 
>