[Mesa-dev] [PATCH 1/2] i965: Allocate vec4_visitor's uniform_size and uniform_vector_size arrays dynamically.
Ian Romanick
idr at freedesktop.org
Fri Nov 22 11:27:07 PST 2013
On 11/22/2013 12:09 AM, Petri Latvala wrote:
> Signed-off-by: Petri Latvala <petri.latvala at 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);
>
More information about the mesa-dev
mailing list