On 4 October 2013 15:44, Eric Anholt <[email protected]> wrote: > Now that both vec4 and fs are dynamically assigning offsets, a lot of the > code is the same. > --- >
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. With that changed, this patch is: Reviewed-by: Paul Berry <[email protected]> I already sent out a comment on patch 4/7. The remainder of the series is: Reviewed-by: Paul Berry <[email protected]> > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
