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

[Dylan Baker]

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 
> Cc: "18.1" 
> ---
>  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-sta...@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?

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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 15:48, Dylan Baker  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 
> > Cc: "18.1" 
> > ---
> >  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-sta...@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


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

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

[Dylan Baker]

Quoting Alex Smith (2018-06-01 07:56:38)
> On 1 June 2018 at 15:48, Dylan Baker  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 
> > Cc: "18.1" 
> > ---
> >  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-sta...@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.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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  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 
 > Cc: "18.1" 
 > ---
 >  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-sta...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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 16:58, Samuel Pitoiset  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  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 
>>>  > Cc: "18.1" 
>>>  > ---
>>>  >  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_f
>>> rom_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-sta...@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. :)
>

What tests are failing?


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

Re: [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 > 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 mailto:dy...@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
mailto:asm...@feralinteractive.com>>
      > Cc: "18.1" mailto:mesa-sta...@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-sta...@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,
      +   

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

[Dylan Baker]

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  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 
> >>  > Cc: "18.1" 
> >>  > ---
> >>  >  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-sta...@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.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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  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  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 
> > >>  > Cc: "18.1" 
> > >>  > ---
> > >>  >  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-sta...@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
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mes