[Mesa-dev] [PATCH 2/2] st/glsl_to_nir: delay adding built-in uniforms to Parameters list
Nicolai Hähnle
nhaehnle at gmail.com
Fri Nov 3 09:59:41 UTC 2017
On 02.11.2017 20:45, Timothy Arceri wrote:
>
>
> 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.
Ah yes, I see it now, thanks. R-b for this patch as well.
Cheers,
Nicolai
>
>
>>
>>
>>> }
>>> return GL_TRUE;
>>>
>>
>>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list