[Mesa-dev] [PATCH] st/glsl: make sure to propagate initialisers to driver storage

[Tapani Pälli]

Verified to fix the test as initializers get propagated;

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

On 5/29/19 6:13 AM, Timothy Arceri wrote:
> This essentially reverts 20234cfe3a20.
> 
> Fixes piglit test:
> tests/spec/arb_get_program_binary/execution/uniform-after-restore.shader_test
> 
> Fixes: 20234cfe3a20 "st/mesa: don't propagate uniforms when restoring from cache"
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110784
> ---
>   src/mesa/program/ir_to_mesa.cpp            | 41 ++++++++++------------
>   src/mesa/program/ir_to_mesa.h              |  3 +-
>   src/mesa/state_tracker/st_glsl_to_nir.cpp  |  2 +-
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  2 +-
>   src/mesa/state_tracker/st_shader_cache.c   |  2 +-
>   5 files changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index f875c00238f..005b855230b 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -2506,8 +2506,7 @@ _mesa_generate_parameters_list_for_uniforms(struct gl_context *ctx,
>   void
>   _mesa_associate_uniform_storage(struct gl_context *ctx,
>                                   struct gl_shader_program *shader_program,
> -                                struct gl_program *prog,
> -                                bool propagate_to_storage)
> +                                struct gl_program *prog)
>   {
>      struct gl_program_parameter_list *params = prog->Parameters;
>      gl_shader_stage shader_type = prog->info.stage;
> @@ -2633,26 +2632,24 @@ _mesa_associate_uniform_storage(struct gl_context *ctx,
>             * data from the linker's backing store.  This will cause values from
>             * initializers in the source code to be copied over.
>             */
> -         if (propagate_to_storage) {
> -            unsigned array_elements = MAX2(1, storage->array_elements);
> -            if (ctx->Const.PackedDriverUniformStorage && !prog->is_arb_asm &&
> -                (storage->is_bindless || !storage->type->contains_opaque())) {
> -               const int dmul = storage->type->is_64bit() ? 2 : 1;
> -               const unsigned components =
> -                  storage->type->vector_elements *
> -                  storage->type->matrix_columns;
> -
> -               for (unsigned s = 0; s < storage->num_driver_storage; s++) {
> -                  gl_constant_value *uni_storage = (gl_constant_value *)
> -                     storage->driver_storage[s].data;
> -                  memcpy(uni_storage, storage->storage,
> -                         sizeof(storage->storage[0]) * components *
> -                         array_elements * dmul);
> -               }
> -            } else {
> -               _mesa_propagate_uniforms_to_driver_storage(storage, 0,
> -                                                          array_elements);
> +         unsigned array_elements = MAX2(1, storage->array_elements);
> +         if (ctx->Const.PackedDriverUniformStorage && !prog->is_arb_asm &&
> +             (storage->is_bindless || !storage->type->contains_opaque())) {
> +            const int dmul = storage->type->is_64bit() ? 2 : 1;
> +            const unsigned components =
> +               storage->type->vector_elements *
> +               storage->type->matrix_columns;
> +
> +            for (unsigned s = 0; s < storage->num_driver_storage; s++) {
> +               gl_constant_value *uni_storage = (gl_constant_value *)
> +                  storage->driver_storage[s].data;
> +               memcpy(uni_storage, storage->storage,
> +                      sizeof(storage->storage[0]) * components *
> +                      array_elements * dmul);
>               }
> +         } else {
> +            _mesa_propagate_uniforms_to_driver_storage(storage, 0,
> +                                                       array_elements);
>            }
>   
>   	      last_location = location;
> @@ -3011,7 +3008,7 @@ get_mesa_program(struct gl_context *ctx,
>       * prog->ParameterValues to get reallocated (e.g., anything that adds a
>       * program constant) has to happen before creating this linkage.
>       */
> -   _mesa_associate_uniform_storage(ctx, shader_program, prog, true);
> +   _mesa_associate_uniform_storage(ctx, shader_program, prog);
>      if (!shader_program->data->LinkStatus) {
>         goto fail_exit;
>      }
> diff --git a/src/mesa/program/ir_to_mesa.h b/src/mesa/program/ir_to_mesa.h
> index f5665e6316e..33eb801bae8 100644
> --- a/src/mesa/program/ir_to_mesa.h
> +++ b/src/mesa/program/ir_to_mesa.h
> @@ -50,8 +50,7 @@ _mesa_generate_parameters_list_for_uniforms(struct gl_context *ctx,
>   void
>   _mesa_associate_uniform_storage(struct gl_context *ctx,
>                                   struct gl_shader_program *shader_program,
> -                                struct gl_program *prog,
> -                                bool propagate_to_storage);
> +                                struct gl_program *prog);
>   
>   #ifdef __cplusplus
>   }
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index 11fc03baf86..b40cb9223df 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -524,7 +524,7 @@ st_glsl_to_nir_post_opts(struct st_context *st, struct gl_program *prog,
>       * prog->ParameterValues to get reallocated (e.g., anything that adds a
>       * program constant) has to happen before creating this linkage.
>       */
> -   _mesa_associate_uniform_storage(st->ctx, shader_program, prog, true);
> +   _mesa_associate_uniform_storage(st->ctx, shader_program, prog);
>   
>      st_set_prog_affected_state_flags(prog);
>   
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index f2344703d71..18a5571aaa8 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -7247,7 +7247,7 @@ get_mesa_program_tgsi(struct gl_context *ctx,
>       * prog->ParameterValues to get reallocated (e.g., anything that adds a
>       * program constant) has to happen before creating this linkage.
>       */
> -   _mesa_associate_uniform_storage(ctx, shader_program, prog, true);
> +   _mesa_associate_uniform_storage(ctx, shader_program, prog);
>      if (!shader_program->data->LinkStatus) {
>         free_glsl_to_tgsi_visitor(v);
>         _mesa_reference_program(ctx, &shader->Program, NULL);
> diff --git a/src/mesa/state_tracker/st_shader_cache.c b/src/mesa/state_tracker/st_shader_cache.c
> index b18829754cb..ae1602310db 100644
> --- a/src/mesa/state_tracker/st_shader_cache.c
> +++ b/src/mesa/state_tracker/st_shader_cache.c
> @@ -366,7 +366,7 @@ st_deserialise_ir_program(struct gl_context *ctx,
>      }
>   
>      st_set_prog_affected_state_flags(prog);
> -   _mesa_associate_uniform_storage(ctx, shProg, prog, false);
> +   _mesa_associate_uniform_storage(ctx, shProg, prog);
>   
>      /* Create Gallium shaders now instead of on demand. */
>      if (ST_DEBUG & DEBUG_PRECOMPILE ||
>