[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 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. :)

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