[Mesa-dev] [PATCH 7/7] i965: Move the common binding table offset code to brw_shader.cpp.
Paul Berry
stereotype441 at gmail.com
Mon Oct 14 14:37:47 PDT 2013
On 4 October 2013 15:44, Eric Anholt <eric at anholt.net> 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 <stereotype441 at gmail.com>
I already sent out a comment on patch 4/7. The remainder of the series is:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131014/7666a701/attachment.html>
More information about the mesa-dev
mailing list