[Mesa-dev] [PATCH 2/2] st/glsl_to_nir: delay adding built-in uniforms to Parameters list

Timothy Arceri tarceri at itsqueeze.com
Thu Nov 2 19:45:36 UTC 2017



On 03/11/17 03:25, Nicolai Hähnle wrote:
> On 01.11.2017 06:20, Timothy Arceri wrote:
>> Delaying adding built-in uniforms until after we convert to NIR
>> gives us a better chance to optimise them away. Also NIR allows
>> us to iterate over the uniforms directly so should be faster.
>> ---
>>   src/mesa/state_tracker/st_glsl_to_nir.cpp  | 68 
>> +++++++++++++++---------------
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  2 +-
>>   2 files changed, 34 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
>> b/src/mesa/state_tracker/st_glsl_to_nir.cpp
>> index 5b37d2cd63..bbef830a2e 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
>> @@ -235,20 +235,53 @@ st_glsl_to_nir(struct st_context *st, struct 
>> gl_program *prog,
>>      options = (const nir_shader_compiler_options *)
>>         pscreen->get_compiler_options(pscreen, PIPE_SHADER_IR_NIR, 
>> ptarget);
>>      assert(options);
>>      if (prog->nir)
>>         return prog->nir;
>>      nir = glsl_to_nir(shader_program, stage, options);
>> +   /* Make a pass over the IR to add state references for any built-in
>> +    * uniforms that are used.  This has to be done now (during linking).
>> +    * Code generation doesn't happen until the first time this shader is
>> +    * used for rendering.  Waiting until then to generate the 
>> parameters is
>> +    * too late.  At that point, the values for the built-in uniforms 
>> won't
>> +    * get sent to the shader.
>> +    */
>> +   nir_foreach_variable(var, &nir->uniforms) {
>> +      if (strncmp(var->name, "gl_", 3) == 0) {
>> +         const nir_state_slot *const slots = var->state_slots;
>> +         assert(var->state_slots != NULL);
>> +
>> +         for (unsigned int i = 0; i < var->num_state_slots; i++) {
>> +            _mesa_add_state_reference(prog->Parameters,
>> +                                      (gl_state_index 
>> *)slots[i].tokens);
>> +         }
>> +      }
>> +   }
>> +
>> +   /* Avoid reallocation of the program parameter list, because the 
>> uniform
>> +    * storage is only associated with the original parameter list.
>> +    * This should be enough for Bitmap and DrawPixels constants.
>> +    */
>> +   _mesa_reserve_parameter_storage(prog->Parameters, 8);
>> +
>> +   /* This has to be done last.  Any operation the can cause
>> +    * 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);
>> +
>> +   st_set_prog_affected_state_flags(prog);
>> +
>>      NIR_PASS_V(nir, nir_lower_io_to_temporaries,
>>            nir_shader_get_entrypoint(nir),
>>            true, true);
>>      NIR_PASS_V(nir, nir_lower_global_vars_to_local);
>>      NIR_PASS_V(nir, nir_split_var_copies);
>>      NIR_PASS_V(nir, nir_lower_var_copies);
>>      NIR_PASS_V(nir, st_nir_lower_builtin);
>>      NIR_PASS_V(nir, nir_lower_atomics, shader_program);
>>      /* fragment shaders may need : */
>> @@ -381,67 +414,32 @@ st_nir_get_mesa_program(struct gl_context *ctx,
>>      prog = shader->Program;
>>      prog->Parameters = _mesa_new_parameter_list();
>>      do_set_program_inouts(shader->ir, prog, shader->Stage);
>>      _mesa_copy_linked_program_data(shader_program, shader);
>>      _mesa_generate_parameters_list_for_uniforms(ctx, shader_program, 
>> shader,
>>                                                  prog->Parameters);
>> -   /* Make a pass over the IR to add state references for any built-in
>> -    * uniforms that are used.  This has to be done now (during linking).
>> -    * Code generation doesn't happen until the first time this shader is
>> -    * used for rendering.  Waiting until then to generate the 
>> parameters is
>> -    * too late.  At that point, the values for the built-in uniforms 
>> won't
>> -    * get sent to the shader.
>> -    */
>> -   foreach_in_list(ir_instruction, node, shader->ir) {
>> -      ir_variable *var = node->as_variable();
>> -
>> -      if ((var == NULL) || (var->data.mode != ir_var_uniform) ||
>> -          (strncmp(var->name, "gl_", 3) != 0))
>> -         continue;
>> -
>> -      const ir_state_slot *const slots = var->get_state_slots();
>> -      assert(slots != NULL);
>> -
>> -      for (unsigned int i = 0; i < var->get_num_state_slots(); i++) {
>> -         _mesa_add_state_reference(prog->Parameters,
>> -                                   (gl_state_index *) slots[i].tokens);
>> -      }
>> -   }
>> -
>>      if (ctx->_Shader->Flags & GLSL_DUMP) {
>>         _mesa_log("\n");
>>         _mesa_log("GLSL IR for linked %s program %d:\n",
>>                _mesa_shader_stage_to_string(shader->Stage),
>>                shader_program->Name);
>>         _mesa_print_ir(_mesa_get_log_file(), shader->ir, NULL);
>>         _mesa_log("\n\n");
>>      }
>>      prog->ExternalSamplersUsed = gl_external_samplers(prog);
>>      _mesa_update_shader_textures_used(shader_program, prog);
>> -   /* Avoid reallocation of the program parameter list, because the 
>> uniform
>> -    * storage is only associated with the original parameter list.
>> -    * This should be enough for Bitmap and DrawPixels constants.
>> -    */
>> -   _mesa_reserve_parameter_storage(prog->Parameters, 8);
>> -
>> -   /* This has to be done last.  Any operation the can cause
>> -    * 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);
>> -
>>      struct st_vertex_program *stvp;
>>      struct st_common_program *stp;
>>      struct st_fragment_program *stfp;
>>      struct st_compute_program *stcp;
>>      switch (shader->Stage) {
>>      case MESA_SHADER_VERTEX:
>>         stvp = (struct st_vertex_program *)prog;
>>         stvp->shader_program = shader_program;
>>         break;
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index eaed052a17..419183d5c9 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -6969,24 +6969,24 @@ st_link_shader(struct gl_context *ctx, struct 
>> gl_shader_program *prog)
>>            pipe_shader_type_from_mesa(shader->Stage);
>>         enum pipe_shader_ir preferred_ir = (enum pipe_shader_ir)
>>            pscreen->get_shader_param(pscreen, ptarget,
>>                                      PIPE_SHADER_CAP_PREFERRED_IR);
>>         struct gl_program *linked_prog = NULL;
>>         if (preferred_ir == PIPE_SHADER_IR_NIR) {
>>            linked_prog = st_nir_get_mesa_program(ctx, prog, shader);
>>         } else {
>>            linked_prog = get_mesa_program_tgsi(ctx, prog, shader);
>> +         st_set_prog_affected_state_flags(linked_prog);
>>         }
>>         if (linked_prog) {
>> -         st_set_prog_affected_state_flags(linked_prog);
>>            if (!ctx->Driver.ProgramStringNotify(ctx,
>>                                                 
>> _mesa_shader_stage_to_program(i),
>>                                                 linked_prog)) {
>>               _mesa_reference_program(ctx, &shader->Program, NULL);
>>               return GL_FALSE;
>>            }
>>         }
> 
> This hunk doesn't seem to fit with the rest of the patch.

It checks if the paramlist is empty or not so we need to call it later 
also. I can update the commit message if that would help.


> 
> 
>>      }
>>      return GL_TRUE;
>>
> 
> 


More information about the mesa-dev mailing list