<div dir="ltr">On 4 October 2013 15:44, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Now that both vec4 and fs are dynamically assigning offsets, a lot of the code is the same. --- </blockquote><div> </div><div>Since next_binding_table_offset is only used to into assign_common_binding_table_offsets(), I'd prefer to see it made into a function argument rather than a class member. That way it wouldn't be necessary to grep through the code to verify that no one else uses it. </div><div>With that changed, this patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div>I already sent out a comment on patch 4/7. The remainder of the series is: </div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> src/mesa/drivers/dri/i965/brw_fs.cpp | 33 ++---------------- src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 3 ++ src/mesa/drivers/dri/i965/brw_shader.cpp | 47 ++++++++++++++++++++++++++ src/mesa/drivers/dri/i965/brw_shader.h | 5 +++ src/mesa/drivers/dri/i965/brw_vec4.cpp | 33 +----------------- src/mesa/drivers/dri/i965/brw_vec4.h | 1 - src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 2 ++ 7 files changed, 61 insertions(+), 63 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 86ff378..13c6ddc 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -2943,37 +2943,10 @@ fs_visitor::setup_payload_gen6() void fs_visitor::assign_binding_table_offsets() { - int num_textures = _mesa_fls(fp->Base.SamplersUsed); - int next = 0; + c->prog_data.binding_table.render_target_start = next_binding_table_offset; + next_binding_table_offset += c->key.nr_color_regions; - c->prog_data.binding_table.render_target_start = next; - next += c->key.nr_color_regions; - - c->prog_data.base.binding_table.texture_start = next; - next += num_textures; - - if (shader) { - c->prog_data.base.binding_table.ubo_start = next; - next += shader->base.NumUniformBlocks; - } - - if (INTEL_DEBUG & DEBUG_SHADER_TIME) { - c->prog_data.base.binding_table.shader_time_start = next; - next++; - } - - if (fp->Base.UsesGather) { - c->prog_data.base.binding_table.gather_texture_start = next; - next += num_textures; - } - - /* This may or may not be used depending on how the compile goes. */ - c->prog_data.base.binding_table.pull_constants_start = next; - next++; - - assert(next < BRW_MAX_SURFACES); - - /* c->prog_data.base.binding_table.size will be set by mark_surface_used. */ + assign_common_binding_table_offsets(); } bool diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index 8fa7f9d..aa76231 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -2617,8 +2617,10 @@ fs_visitor::fs_visitor(struct brw_context *brw, this->c = c; this->brw = brw; this->fp = fp; + this->prog = &fp->Base; this->shader_prog = shader_prog; this->prog = &fp->Base; + this->stage_prog_data = &c->prog_data.base; this->ctx = &brw->ctx; this->mem_ctx = ralloc_context(NULL); if (shader_prog) @@ -2651,6 +2653,7 @@ fs_visitor::fs_visitor(struct brw_context *brw, this->force_uncompressed_stack = 0; this->force_sechalf_stack = 0; + this->next_binding_table_offset = 0; memset(&this->param_size, 0, sizeof(this->param_size)); } diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp index 61c4bf5..b97bb5e 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp @@ -578,3 +578,50 @@ backend_visitor::dump_instructions() dump_instruction(inst); } } + + +/** + * Sets up the starting offsets for the groups of binding table entries + * commong to all pipeline stages. + * + * Unused groups are initialized to 0xd0d0d0d0 to make it obvious that they're + * unused but also make sure that addition of small offsets to them will + * trigger some of our asserts that surface indices are < BRW_MAX_SURFACES. + */ +void +backend_visitor::assign_common_binding_table_offsets() +{ + int num_textures = _mesa_fls(prog->SamplersUsed); + + stage_prog_data->binding_table.texture_start = next_binding_table_offset; + next_binding_table_offset += num_textures; + + if (shader) { + stage_prog_data->binding_table.ubo_start = next_binding_table_offset; + next_binding_table_offset += shader->base.NumUniformBlocks; + } else { + stage_prog_data->binding_table.ubo_start = 0xd0d0d0d0; + } + + if (INTEL_DEBUG & DEBUG_SHADER_TIME) { + stage_prog_data->binding_table.shader_time_start = next_binding_table_offset; + next_binding_table_offset++; + } else { + stage_prog_data->binding_table.shader_time_start = 0xd0d0d0d0; + } + + if (prog->UsesGather) { + stage_prog_data->binding_table.gather_texture_start = next_binding_table_offset; + next_binding_table_offset += num_textures; + } else { + stage_prog_data->binding_table.gather_texture_start = 0xd0d0d0d0; + } + + /* This may or may not be used depending on how the compile goes. */ + stage_prog_data->binding_table.pull_constants_start = next_binding_table_offset; + next_binding_table_offset++; + + assert(next_binding_table_offset <= BRW_MAX_SURFACES); + + /* prog_data->base.binding_table.size will be set by mark_surface_used. */ +} diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h index 41ab03a..85fedc2 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.h +++ b/src/mesa/drivers/dri/i965/brw_shader.h @@ -60,6 +60,9 @@ public: struct brw_shader *shader; struct gl_shader_program *shader_prog; struct gl_program *prog; + struct brw_stage_prog_data *stage_prog_data; + + uint32_t next_binding_table_offset; /** ralloc context for temporary data used during compile */ void *mem_ctx; @@ -72,6 +75,8 @@ public: virtual void dump_instruction(backend_instruction *inst) = 0; void dump_instructions(); + + void assign_common_binding_table_offsets(); }; uint32_t brw_texture_offset(ir_constant *offset); diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp index 43ec180..49c0127 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp @@ -1405,37 +1405,6 @@ vec4_visitor::emit_shader_time_write(enum shader_time_shader_type type, emit(SHADER_OPCODE_SHADER_TIME_ADD, dst_reg(), src_reg(dst)); } -void -vec4_visitor::assign_binding_table_offsets() -{ - int num_textures = _mesa_fls(prog->SamplersUsed); - int next = 0; - - prog_data->base.binding_table.texture_start = next; - next += num_textures; - - if (shader) { - prog_data->base.binding_table.ubo_start = next; - next += shader->base.NumUniformBlocks; - } - - if (INTEL_DEBUG & DEBUG_SHADER_TIME) { - prog_data->base.binding_table.shader_time_start = next; - next++; - } - - if (prog->UsesGather) { - prog_data->base.binding_table.gather_texture_start = next; - next += num_textures; - } - - /* This may or may not be used depending on how the compile goes. */ - prog_data->base.binding_table.pull_constants_start = next; - next++; - - /* prog_data->base.binding_table.size will be set by mark_surface_used. */ -} - bool vec4_visitor::run() { @@ -1444,7 +1413,7 @@ vec4_visitor::run() if (INTEL_DEBUG & DEBUG_SHADER_TIME) emit_shader_time_begin(); - assign_binding_table_offsets(); + assign_common_binding_table_offsets(); emit_prolog(); diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h index d75c46a..cebf573 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.h +++ b/src/mesa/drivers/dri/i965/brw_vec4.h @@ -332,7 +332,6 @@ public: bool run(void); void fail(const char *msg, ...); - void assign_binding_table_offsets(); int virtual_grf_alloc(int size); void setup_uniform_clipplane_values(); void setup_uniform_values(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 150bc31..5ecb6d2 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -3121,6 +3121,7 @@ vec4_visitor::vec4_visitor(struct brw_context *brw, this->prog = prog; this->key = key; this->prog_data = prog_data; + this->stage_prog_data = &prog_data->base; this->variable_ht = hash_table_ctor(0, hash_table_pointer_hash, @@ -3138,6 +3139,7 @@ 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->next_binding_table_offset = 0; } vec4_visitor::~vec4_visitor() -- 1.8.4.rc3 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>