[Mesa-dev] [PATCH 24/25] mesa/glsl: set and get cs layouts to and from shader_info

[Lionel Landwerlin]

On 09/01/17 05:13, Timothy Arceri wrote:
> ---
>   src/compiler/glsl/linker.cpp | 35 +++++++++++++++--------------------
>   src/mesa/main/mtypes.h       | 10 ----------
>   src/mesa/main/shaderapi.c    |  6 ++----
>   src/mesa/main/shaderobj.c    |  2 --
>   4 files changed, 17 insertions(+), 36 deletions(-)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 53ee7e6..f822778 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -2005,21 +2005,21 @@ link_gs_inout_layout_qualifiers(struct gl_shader_program *prog,
>    */
>   static void
>   link_cs_input_layout_qualifiers(struct gl_shader_program *prog,
> -                                struct gl_linked_shader *linked_shader,
> +                                struct gl_program *gl_prog,
>                                   struct gl_shader **shader_list,
>                                   unsigned num_shaders)
>   {
> -   for (int i = 0; i < 3; i++)
> -      linked_shader->info.Comp.LocalSize[i] = 0;
> -
> -   linked_shader->info.Comp.LocalSizeVariable = false;
> -
>      /* This function is called for all shader stages, but it only has an effect
>       * for compute shaders.
>       */
> -   if (linked_shader->Stage != MESA_SHADER_COMPUTE)
> +   if (gl_prog->info.stage != MESA_SHADER_COMPUTE)
>         return;
>   
> +   for (int i = 0; i < 3; i++)
> +      gl_prog->info.cs.local_size[i] = 0;
> +
> +   gl_prog->info.cs.local_size_variable = false;
> +
>      /* From the ARB_compute_shader spec, in the section describing local size
>       * declarations:
>       *
> @@ -2034,9 +2034,9 @@ link_cs_input_layout_qualifiers(struct gl_shader_program *prog,
>         struct gl_shader *shader = shader_list[sh];
>   
>         if (shader->info.Comp.LocalSize[0] != 0) {
> -         if (linked_shader->info.Comp.LocalSize[0] != 0) {
> +         if (gl_prog->info.cs.local_size[0] != 0) {
>               for (int i = 0; i < 3; i++) {
> -               if (linked_shader->info.Comp.LocalSize[i] !=
> +               if (gl_prog->info.cs.local_size[i] !=
>                      shader->info.Comp.LocalSize[i]) {
>                     linker_error(prog, "compute shader defined with conflicting "
>                                  "local sizes\n");
> @@ -2045,11 +2045,11 @@ link_cs_input_layout_qualifiers(struct gl_shader_program *prog,
>               }
>            }
>            for (int i = 0; i < 3; i++) {
> -            linked_shader->info.Comp.LocalSize[i] =
> +            gl_prog->info.cs.local_size[i] =
>                  shader->info.Comp.LocalSize[i];
>            }
>         } else if (shader->info.Comp.LocalSizeVariable) {
> -         if (linked_shader->info.Comp.LocalSize[0] != 0) {
> +         if (gl_prog->info.cs.local_size[0] != 0) {
>               /* The ARB_compute_variable_group_size spec says:
>                *
>                *     If one compute shader attached to a program declares a
> @@ -2061,7 +2061,7 @@ link_cs_input_layout_qualifiers(struct gl_shader_program *prog,
>                            "variable local group size\n");
>               return;
>            }
> -         linked_shader->info.Comp.LocalSizeVariable = true;
> +         gl_prog->info.cs.local_size_variable = true;
>         }
>      }
>   
> @@ -2069,17 +2069,12 @@ link_cs_input_layout_qualifiers(struct gl_shader_program *prog,
>       * since we already know we're in the right type of shader program
>       * for doing it.
>       */
> -   if (linked_shader->info.Comp.LocalSize[0] == 0 &&
> -       !linked_shader->info.Comp.LocalSizeVariable) {
> +   if (gl_prog->info.cs.local_size[0] == 0 &&
> +       !gl_prog->info.cs.local_size_variable) {
>         linker_error(prog, "compute shader must contain a fixed or a variable "
>                            "local group size\n");
>         return;
>      }
> -   for (int i = 0; i < 3; i++)
> -      prog->Comp.LocalSize[i] = linked_shader->info.Comp.LocalSize[i];
> -
> -   prog->Comp.LocalSizeVariable =
> -      linked_shader->info.Comp.LocalSizeVariable;
>   }
>   
>   
> @@ -2209,7 +2204,7 @@ link_intrastage_shaders(void *mem_ctx,
>      link_tcs_out_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
>      link_tes_in_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
>      link_gs_inout_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
> -   link_cs_input_layout_qualifiers(prog, linked, shader_list, num_shaders);
> +   link_cs_input_layout_qualifiers(prog, gl_prog, shader_list, num_shaders);
>      link_xfb_stride_layout_qualifiers(ctx, prog, linked, shader_list,
>                                        num_shaders);
>   
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 2004720..aff426f 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2760,19 +2760,9 @@ struct gl_shader_program
>       */
>      struct {
>         /**
> -       * If this shader contains a compute stage, size specified using
> -       * local_size_{x,y,z}.  Otherwise undefined.
> -       */
> -      unsigned LocalSize[3];
> -      /**
>          * Size of shared variables accessed by the compute shader.
>          */
>         unsigned SharedSize;

Do you think we could get rid SharedSize too and store it directly into 
shader_info?

> -
> -      /**
> -       * Whether a variable work group size has been specified.
> -       */
> -      bool LocalSizeVariable;
>      } Comp;
>   
>      /** Data shared by gl_program and gl_shader_program */
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 4537ea4..8c6e035 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -807,7 +807,8 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname,
>            return;
>         }
>         for (i = 0; i < 3; i++)
> -         params[i] = shProg->Comp.LocalSize[i];
> +         params[i] = shProg->_LinkedShaders[MESA_SHADER_COMPUTE]->
> +            Program->info.cs.local_size[i];
>         return;
>      }
>      case GL_PROGRAM_SEPARABLE:
> @@ -2214,10 +2215,7 @@ _mesa_copy_linked_program_data(const struct gl_shader_program *src,
>         break;
>      }
>      case MESA_SHADER_COMPUTE: {
> -      for (int i = 0; i < 3; i++)
> -         dst->info.cs.local_size[i] = src->Comp.LocalSize[i];
>         dst->info.cs.shared_size = src->Comp.SharedSize;
> -      dst->info.cs.local_size_variable = src->Comp.LocalSizeVariable;
>         break;
>      }
>      default:
> diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
> index 4e514a3..b41137f 100644
> --- a/src/mesa/main/shaderobj.c
> +++ b/src/mesa/main/shaderobj.c
> @@ -282,8 +282,6 @@ init_shader_program(struct gl_shader_program *prog)
>      prog->Geom.UsesEndPrimitive = false;
>      prog->Geom.UsesStreams = false;
>   
> -   prog->Comp.LocalSizeVariable = false;
> -
>      prog->TransformFeedback.BufferMode = GL_INTERLEAVED_ATTRIBS;
>   
>      exec_list_make_empty(&prog->EmptyUniformLocations);