On 11/22/2013 12:09 AM, Petri Latvala wrote: > Signed-off-by: Petri Latvala <petri.latv...@intel.com> > --- > src/mesa/drivers/dri/i965/brw_vec4.cpp | 5 +++-- > src/mesa/drivers/dri/i965/brw_vec4.h | 14 +++++++++++--- > src/mesa/drivers/dri/i965/brw_vec4_gs.c | 2 +- > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 12 +++++++----- > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h | 6 ++++-- > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 7 ++++++- > src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 6 ++++-- > src/mesa/drivers/dri/i965/brw_vs.c | 2 +- > src/mesa/drivers/dri/i965/brw_vs.h | 6 ++++-- > 9 files changed, 41 insertions(+), 19 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp > b/src/mesa/drivers/dri/i965/brw_vec4.cpp > index 73f91a0..3da1b4d 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp > @@ -1558,7 +1558,8 @@ brw_vs_emit(struct brw_context *brw, > struct brw_vs_compile *c, > struct brw_vs_prog_data *prog_data, > void *mem_ctx, > - unsigned *final_assembly_size) > + unsigned *final_assembly_size, > + unsigned param_count) > { > bool start_busy = false; > float start_time = 0; > @@ -1585,7 +1586,7 @@ brw_vs_emit(struct brw_context *brw, > } > } > > - vec4_vs_visitor v(brw, c, prog_data, prog, shader, mem_ctx); > + vec4_vs_visitor v(brw, c, prog_data, prog, shader, mem_ctx, param_count); > if (!v.run()) { > if (prog) { > prog->LinkStatus = false; > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h > b/src/mesa/drivers/dri/i965/brw_vec4.h > index 5cec9f9..552ca55 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.h > +++ b/src/mesa/drivers/dri/i965/brw_vec4.h > @@ -231,7 +231,8 @@ public: > struct brw_shader *shader, > void *mem_ctx, > bool debug_flag, > - bool no_spills); > + bool no_spills, > + unsigned param_count); > ~vec4_visitor(); > > dst_reg dst_null_f() > @@ -325,8 +326,9 @@ public: > */ > dst_reg output_reg[BRW_VARYING_SLOT_COUNT]; > const char *output_reg_annotation[BRW_VARYING_SLOT_COUNT]; > - int uniform_size[MAX_UNIFORMS]; > - int uniform_vector_size[MAX_UNIFORMS]; > + unsigned uniform_param_count; > + int* uniform_size; > + int* uniform_vector_size; > int uniforms; > > src_reg shader_start_time; > @@ -546,6 +548,12 @@ private: > * If true, then register allocation should fail instead of spilling. > */ > const bool no_spills; > + > + /* > + * Make noncopyable. > + */ > + vec4_visitor(const vec4_visitor&); > + vec4_visitor& operator=(const vec4_visitor&);
I think this should be a separate patch with it's own justification. I'm also not sure it's strictly necessary. I'm not a C++ expert (and most people on the project aren't either), so bear with me a bit. The usual reason to make a class non-copyable is because the default copy-constructor will only make a shallow copy of pointer fields. This can lead to either double-frees or dereferencing freed memory. However, since we're using ralloc, and the first construction of a vec4_visitor object will out-live any possible copies, neither of these situations can occur. Is that a fair assessment? Now... we probably should do this anyway... and there are a lot of classes in the i965 back-end where this should occur. I don't know if we want to make a macro to generate this boiler-plate code or if we just want to manually add it to every class. Perhaps Ken, Paul, or Curro will have an opinion... > }; > > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c > b/src/mesa/drivers/dri/i965/brw_vec4_gs.c > index b52d646..b0b77d1 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c > @@ -213,7 +213,7 @@ do_gs_prog(struct brw_context *brw, > void *mem_ctx = ralloc_context(NULL); > unsigned program_size; > const unsigned *program = > - brw_gs_emit(brw, prog, &c, mem_ctx, &program_size); > + brw_gs_emit(brw, prog, &c, mem_ctx, &program_size, param_count); > if (program == NULL) { > ralloc_free(mem_ctx); > return false; > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > index adbb1cf..16c05f5 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > @@ -38,10 +38,11 @@ vec4_gs_visitor::vec4_gs_visitor(struct brw_context *brw, > struct gl_shader_program *prog, > struct brw_shader *shader, > void *mem_ctx, > - bool no_spills) > + bool no_spills, > + unsigned param_count) > : vec4_visitor(brw, &c->base, &c->gp->program.Base, &c->key.base, > &c->prog_data.base, prog, shader, mem_ctx, > - INTEL_DEBUG & DEBUG_GS, no_spills), > + INTEL_DEBUG & DEBUG_GS, no_spills, param_count), > c(c) > { > } > @@ -539,7 +540,8 @@ brw_gs_emit(struct brw_context *brw, > struct gl_shader_program *prog, > struct brw_gs_compile *c, > void *mem_ctx, > - unsigned *final_assembly_size) > + unsigned *final_assembly_size, > + unsigned param_count) > { > struct brw_shader *shader = > (brw_shader *) prog->_LinkedShaders[MESA_SHADER_GEOMETRY]; > @@ -556,7 +558,7 @@ brw_gs_emit(struct brw_context *brw, > if (likely(!(INTEL_DEBUG & DEBUG_NO_DUAL_OBJECT_GS))) { > c->prog_data.dual_instanced_dispatch = false; > > - vec4_gs_visitor v(brw, c, prog, shader, mem_ctx, true /* no_spills */); > + vec4_gs_visitor v(brw, c, prog, shader, mem_ctx, true /* no_spills */, > param_count); > if (v.run()) { > vec4_generator g(brw, prog, &c->gp->program.Base, > &c->prog_data.base, > mem_ctx, INTEL_DEBUG & DEBUG_GS); > @@ -579,7 +581,7 @@ brw_gs_emit(struct brw_context *brw, > */ > c->prog_data.dual_instanced_dispatch = true; > > - vec4_gs_visitor v(brw, c, prog, shader, mem_ctx, false /* no_spills */); > + vec4_gs_visitor v(brw, c, prog, shader, mem_ctx, false /* no_spills */, > param_count); > if (!v.run()) { > prog->LinkStatus = false; > ralloc_strcat(&prog->InfoLog, v.fail_msg); > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h > b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h > index 68756f7..b28e4de 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h > @@ -65,7 +65,8 @@ const unsigned *brw_gs_emit(struct brw_context *brw, > struct gl_shader_program *prog, > struct brw_gs_compile *c, > void *mem_ctx, > - unsigned *final_assembly_size); > + unsigned *final_assembly_size, > + unsigned param_count); > > #ifdef __cplusplus > } /* extern "C" */ > @@ -82,7 +83,8 @@ public: > struct gl_shader_program *prog, > struct brw_shader *shader, > void *mem_ctx, > - bool no_spills); > + bool no_spills, > + unsigned param_count); > > protected: > virtual dst_reg *make_reg_for_system_value(ir_variable *ir); > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > index a13eafb..df38dab 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > @@ -3248,11 +3248,13 @@ vec4_visitor::vec4_visitor(struct brw_context *brw, > struct brw_shader *shader, > void *mem_ctx, > bool debug_flag, > - bool no_spills) > + bool no_spills, > + unsigned param_count) > : sanity_param_count(0), > fail_msg(NULL), > first_non_payload_grf(0), > need_all_constants_in_pull_buffer(false), > + uniform_param_count(param_count), > debug_flag(debug_flag), > no_spills(no_spills) > { > @@ -3290,6 +3292,9 @@ vec4_visitor::vec4_visitor(struct brw_context *brw, > this->max_grf = brw->gen >= 7 ? GEN7_MRF_HACK_START : BRW_MAX_GRF; > > this->uniforms = 0; > + > + this->uniform_size = rzalloc_array(mem_ctx, int, > this->uniform_param_count); > + this->uniform_vector_size = rzalloc_array(mem_ctx, int, > this->uniform_param_count); > } > > vec4_visitor::~vec4_visitor() > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp > index 31c42c4..a653de1 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp > @@ -212,10 +212,12 @@ vec4_vs_visitor::vec4_vs_visitor(struct brw_context > *brw, > struct brw_vs_prog_data *vs_prog_data, > struct gl_shader_program *prog, > struct brw_shader *shader, > - void *mem_ctx) > + void *mem_ctx, > + unsigned param_count) > : vec4_visitor(brw, &vs_compile->base, &vs_compile->vp->program.Base, > &vs_compile->key.base, &vs_prog_data->base, prog, shader, > - mem_ctx, INTEL_DEBUG & DEBUG_VS, false /* no_spills */), > + mem_ctx, INTEL_DEBUG & DEBUG_VS, false /* no_spills */, > + param_count), > vs_compile(vs_compile), > vs_prog_data(vs_prog_data) > { > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c > b/src/mesa/drivers/dri/i965/brw_vs.c > index b5c8b63..1f90c32 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs.c > +++ b/src/mesa/drivers/dri/i965/brw_vs.c > @@ -288,7 +288,7 @@ do_vs_prog(struct brw_context *brw, > > /* Emit GEN4 code. > */ > - program = brw_vs_emit(brw, prog, &c, &prog_data, mem_ctx, &program_size); > + program = brw_vs_emit(brw, prog, &c, &prog_data, mem_ctx, &program_size, > param_count); > if (program == NULL) { > ralloc_free(mem_ctx); > return false; > diff --git a/src/mesa/drivers/dri/i965/brw_vs.h > b/src/mesa/drivers/dri/i965/brw_vs.h > index 5d62e47..59b48a1 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs.h > +++ b/src/mesa/drivers/dri/i965/brw_vs.h > @@ -88,7 +88,8 @@ const unsigned *brw_vs_emit(struct brw_context *brw, > struct brw_vs_compile *c, > struct brw_vs_prog_data *prog_data, > void *mem_ctx, > - unsigned *program_size); > + unsigned *program_size, > + unsigned param_count); > bool brw_vs_precompile(struct gl_context *ctx, struct gl_shader_program > *prog); > void brw_vs_debug_recompile(struct brw_context *brw, > struct gl_shader_program *prog, > @@ -110,7 +111,8 @@ public: > struct brw_vs_prog_data *vs_prog_data, > struct gl_shader_program *prog, > struct brw_shader *shader, > - void *mem_ctx); > + void *mem_ctx, > + unsigned param_count); > > protected: > virtual dst_reg *make_reg_for_system_value(ir_variable *ir); > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev