[Mesa-dev] [PATCH 5/7] glsl/mesa: split gl_shader in two
Timothy Arceri
timothy.arceri at collabora.com
Wed Jun 29 12:26:54 UTC 2016
On Wed, 2016-06-29 at 13:57 +0200, Iago Toral wrote:
> On Tue, 2016-06-28 at 11:52 +1000, Timothy Arceri wrote:
> > There are two distinctly different uses of this struct. The first
> > is to store GL shader objects. The second is to store information
> > about a shader stage thats been linked.
> >
> > The two uses actually share few fields and there is clearly
> > confusion
> > about their use. For example the linked shaders map one to one with
> > a program so can simply be destroyed along with the program.
> > However
> > previously we were calling reference counting on the linked
> > shaders.
> >
> > We were also creating linked shaders with a name even though it
> > is always 0 and called the driver version of the _mesa_new_shader()
> > function unnecessarily for GL shader objects.
> > ---
> > src/compiler/glsl/glsl_to_nir.cpp | 2 +-
> > src/compiler/glsl/ir_optimization.h | 23 ++--
> > src/compiler/glsl/link_atomics.cpp | 2 +-
> > src/compiler/glsl/link_functions.cpp | 8 +-
> > src/compiler/glsl/link_interface_blocks.cpp | 10 +-
> > src/compiler/glsl/link_uniform_blocks.cpp | 2 +-
> > src/compiler/glsl/link_uniform_initializers.cpp | 6 +-
> > src/compiler/glsl/link_uniforms.cpp | 8 +-
> > src/compiler/glsl/link_varyings.cpp | 17 ++-
> > src/compiler/glsl/link_varyings.h | 17 ++-
> > src/compiler/glsl/linker.cpp | 106 +++++++
> > --------
> > src/compiler/glsl/linker.h | 10 +-
> > src/compiler/glsl/lower_distance.cpp | 3 +-
> > src/compiler/glsl/lower_named_interface_blocks.cpp | 2 +-
> > src/compiler/glsl/lower_packed_varyings.cpp | 6 +-
> > src/compiler/glsl/lower_shared_reference.cpp | 6 +-
> > src/compiler/glsl/lower_tess_level.cpp | 2 +-
> > src/compiler/glsl/lower_ubo_reference.cpp | 6 +-
> > src/compiler/glsl/lower_vector_derefs.cpp | 2 +-
> > src/compiler/glsl/lower_vertex_id.cpp | 2 +-
> > src/compiler/glsl/opt_dead_builtin_varyings.cpp | 11 +-
> > src/compiler/glsl/standalone.cpp | 4 +-
> > src/compiler/glsl/standalone_scaffolding.cpp | 20 +++
> > src/compiler/glsl/standalone_scaffolding.h | 7 +
> > src/compiler/glsl/test_optpass.cpp | 2 +-
> > src/mesa/drivers/common/meta.c | 2 +-
> > src/mesa/drivers/dri/i965/brw_context.c | 5 +-
> > src/mesa/drivers/dri/i965/brw_context.h | 8 +-
> > src/mesa/drivers/dri/i965/brw_gs.c | 5 +-
> > src/mesa/drivers/dri/i965/brw_link.cpp | 24 ++--
> > src/mesa/drivers/dri/i965/brw_program.c | 2 +-
> > src/mesa/drivers/dri/i965/brw_program.h | 2 +-
> > src/mesa/drivers/dri/i965/brw_shader.cpp | 4 +-
> > src/mesa/drivers/dri/i965/brw_shader.h | 3 +-
> > src/mesa/drivers/dri/i965/brw_tcs.c | 2 +-
> > src/mesa/drivers/dri/i965/brw_tes.c | 3 +-
> > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 +-
> > src/mesa/main/api_validate.c | 6 +-
> > src/mesa/main/dd.h | 3 +-
> > src/mesa/main/ff_fragment_shader.cpp | 2 +-
> > src/mesa/main/mtypes.h | 143
> > ++++++++++++++++++---
> > src/mesa/main/shader_query.cpp | 2 +-
> > src/mesa/main/shaderapi.c | 19 +--
> > src/mesa/main/shaderobj.c | 32 ++++-
> > src/mesa/main/shaderobj.h | 7 +
> > src/mesa/main/uniform_query.cpp | 4 +-
> > src/mesa/main/uniforms.c | 2 +-
> > src/mesa/program/ir_to_mesa.cpp | 4 +-
> > src/mesa/program/ir_to_mesa.h | 2 +-
> > src/mesa/program/prog_print.c | 14 +-
> > src/mesa/program/prog_print.h | 2 +-
> > src/mesa/state_tracker/st_atom_constbuf.c | 2 +-
> > src/mesa/state_tracker/st_atom_image.c | 2 +-
> > src/mesa/state_tracker/st_atom_storagebuf.c | 2 +-
> > src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
> > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 +-
> > src/mesa/state_tracker/st_nir.h | 2 +-
> > src/mesa/state_tracker/st_program.c | 7 -
> > 58 files changed, 388 insertions(+), 227 deletions(-)
> >
> > diff --git a/src/compiler/glsl/glsl_to_nir.cpp
> > b/src/compiler/glsl/glsl_to_nir.cpp
> > index a22fd5b..0ad3be1 100644
> > --- a/src/compiler/glsl/glsl_to_nir.cpp
> > +++ b/src/compiler/glsl/glsl_to_nir.cpp
> > @@ -134,7 +134,7 @@ glsl_to_nir(const struct gl_shader_program
> > *shader_prog,
> > gl_shader_stage stage,
> > const nir_shader_compiler_options *options)
> > {
> > - struct gl_shader *sh = shader_prog->_LinkedShaders[stage];
> > + struct gl_linked_shader *sh = shader_prog-
> > >_LinkedShaders[stage];
> >
> > nir_shader *shader = nir_shader_create(NULL, stage, options);
> >
> > diff --git a/src/compiler/glsl/ir_optimization.h
> > b/src/compiler/glsl/ir_optimization.h
> > index ba14e34..ba79076 100644
> > --- a/src/compiler/glsl/ir_optimization.h
> > +++ b/src/compiler/glsl/ir_optimization.h
> > @@ -86,7 +86,8 @@ bool do_copy_propagation(exec_list
> > *instructions);
> > bool do_copy_propagation_elements(exec_list *instructions);
> > bool do_constant_propagation(exec_list *instructions);
> > void do_dead_builtin_varyings(struct gl_context *ctx,
> > - gl_shader *producer, gl_shader
> > *consumer,
> > + gl_linked_shader *producer,
> > + gl_linked_shader *consumer,
> > unsigned num_tfeedback_decls,
> > class tfeedback_decl
> > *tfeedback_decls);
> > bool do_dead_code(exec_list *instructions, bool
> > uniform_locations_assigned);
> > @@ -119,26 +120,30 @@ bool
> > lower_variable_index_to_cond_assign(gl_shader_stage stage,
> > bool lower_temp, bool lower_uniform);
> > bool lower_quadop_vector(exec_list *instructions, bool
> > dont_lower_swz);
> > bool lower_const_arrays_to_uniforms(exec_list *instructions);
> > -bool lower_clip_cull_distance(struct gl_shader_program *prog,
> > gl_shader *shader);
> > +bool lower_clip_cull_distance(struct gl_shader_program *prog,
> > + gl_linked_shader *shader);
> > void lower_output_reads(unsigned stage, exec_list *instructions);
> > bool lower_packing_builtins(exec_list *instructions, int op_mask);
> > -void lower_shared_reference(struct gl_shader *shader, unsigned
> > *shared_size);
> > -void lower_ubo_reference(struct gl_shader *shader, bool
> > clamp_block_indices);
> > +void lower_shared_reference(struct gl_linked_shader *shader,
> > + unsigned *shared_size);
> > +void lower_ubo_reference(struct gl_linked_shader *shader,
> > + bool clamp_block_indices);
> > void lower_packed_varyings(void *mem_ctx,
> > unsigned locations_used,
> > ir_variable_mode mode,
> > - unsigned gs_input_vertices, gl_shader
> > *shader,
> > + unsigned gs_input_vertices,
> > + gl_linked_shader *shader,
> > bool disable_varying_packing, bool
> > xfb_enabled);
> > bool lower_vector_insert(exec_list *instructions, bool
> > lower_nonconstant_index);
> > -bool lower_vector_derefs(gl_shader *shader);
> > -void lower_named_interface_blocks(void *mem_ctx, gl_shader
> > *shader);
> > +bool lower_vector_derefs(gl_linked_shader *shader);
> > +void lower_named_interface_blocks(void *mem_ctx, gl_linked_shader
> > *shader);
> > bool optimize_redundant_jumps(exec_list *instructions);
> > bool optimize_split_arrays(exec_list *instructions, bool linked);
> > bool lower_offset_arrays(exec_list *instructions);
> > void optimize_dead_builtin_variables(exec_list *instructions,
> > enum ir_variable_mode other);
> > -bool lower_tess_level(gl_shader *shader);
> > +bool lower_tess_level(gl_linked_shader *shader);
> >
> > -bool lower_vertex_id(gl_shader *shader);
> > +bool lower_vertex_id(gl_linked_shader *shader);
> >
> > bool lower_subroutine(exec_list *instructions, struct
> > _mesa_glsl_parse_state *state);
> > void propagate_invariance(exec_list *instructions);
> > diff --git a/src/compiler/glsl/link_atomics.cpp
> > b/src/compiler/glsl/link_atomics.cpp
> > index 277d473..fa845e9 100644
> > --- a/src/compiler/glsl/link_atomics.cpp
> > +++ b/src/compiler/glsl/link_atomics.cpp
> > @@ -150,7 +150,7 @@ namespace {
> > *num_buffers = 0;
> >
> > for (unsigned i = 0; i < MESA_SHADER_STAGES; ++i) {
> > - struct gl_shader *sh = prog->_LinkedShaders[i];
> > + struct gl_linked_shader *sh = prog->_LinkedShaders[i];
> > if (sh == NULL)
> > continue;
> >
> > diff --git a/src/compiler/glsl/link_functions.cpp
> > b/src/compiler/glsl/link_functions.cpp
> > index c9dacc1..079f3b9 100644
> > --- a/src/compiler/glsl/link_functions.cpp
> > +++ b/src/compiler/glsl/link_functions.cpp
> > @@ -37,7 +37,7 @@ namespace {
> >
> > class call_link_visitor : public ir_hierarchical_visitor {
> > public:
> > - call_link_visitor(gl_shader_program *prog, gl_shader *linked,
> > + call_link_visitor(gl_shader_program *prog, gl_linked_shader
> > *linked,
> > gl_shader **shader_list, unsigned
> > num_shaders)
> > {
> > this->prog = prog;
> > @@ -297,7 +297,7 @@ private:
> > * linked shader that are accessed by the function. It is also
> > used to add
> > * global variables from the shader where the function
> > originated.
> > */
> > - gl_shader *linked;
> > + gl_linked_shader *linked;
> >
> > /**
> > * Table of variables local to the function.
> > @@ -325,7 +325,7 @@ find_matching_signature(const char *name, const
> > exec_list *actual_parameters,
> > * signature that we found isn't a built-in, keep
> > looking. Also keep
> > * looking if we expect a non-built-in but found a built-
> > in.
> > */
> > - if (use_builtin != sig->is_builtin())
> > + if (use_builtin == sig->is_builtin())
>
> Uh, this is what I mentioned in a previous patch, I guess you meant
> to
> put this hunk into that other patch instead.
>
Hi, thanks for reviewing these :) Yes I thought I sent out an email in
reply to patch 4 right after sending the series saying I squashed the
fix into the wrong patch, maybe it didn't get through? Anyway I've
fixed it up locally.
Nice catch by the way.
More information about the mesa-dev
mailing list