On 11/22/2013 11:27 AM, Ian Romanick wrote: > 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...
Or you could just be sensible and just not make copies of things where it makes no sense to do so... I would drop this hunk...it's unrelated and unnecessary. --Ken _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev