On 05/07/2015 06:17 PM, Pohjolainen, Topi wrote: > On Tue, Apr 28, 2015 at 11:08:08PM +0300, Abdiel Janulgue wrote: >> The resource streamer is able to gather and pack sparsely-located >> constant data from any buffer object by a referring to a gather table >> This patch adds support for keeping track of these constant data >> fetches into a gather table. >> >> The gather table is generated from two sources. Ordinary uniform fetches >> are stored first. These are then combined with a separate table containing >> UBO entries. The separate entry for UBOs is needed to make it easier to >> generate the gather mask when combining and packing the constant data. >> >> Signed-off-by: Abdiel Janulgue <abdiel.janul...@linux.intel.com> >> --- >> src/mesa/drivers/dri/i965/brw_context.h | 9 +++++++++ >> src/mesa/drivers/dri/i965/brw_gs.c | 4 ++++ >> src/mesa/drivers/dri/i965/brw_program.c | 5 +++++ >> src/mesa/drivers/dri/i965/brw_shader.cpp | 4 +++- >> src/mesa/drivers/dri/i965/brw_shader.h | 11 +++++++++++ >> src/mesa/drivers/dri/i965/brw_vs.c | 5 +++++ >> src/mesa/drivers/dri/i965/brw_wm.c | 5 +++++ >> 7 files changed, 42 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_context.h >> b/src/mesa/drivers/dri/i965/brw_context.h >> index 7fd49e9..e25c64d 100644 >> --- a/src/mesa/drivers/dri/i965/brw_context.h >> +++ b/src/mesa/drivers/dri/i965/brw_context.h >> @@ -355,9 +355,12 @@ struct brw_stage_prog_data { >> >> GLuint nr_params; /**< number of float params/constants */ >> GLuint nr_pull_params; >> + GLuint nr_ubo_params; >> + GLuint nr_gather_table; > > I would introduce these as non gl-types - we are trying to move away from > them. Perhaps change "nr_params" and "nr_pull_params" while you are at it. > >> >> unsigned curb_read_length; >> unsigned total_scratch; >> + unsigned max_ubo_const_block; >> >> /** >> * Register where the thread expects to find input data from the URB >> @@ -375,6 +378,12 @@ struct brw_stage_prog_data { >> */ >> const gl_constant_value **param; >> const gl_constant_value **pull_param; >> + struct { >> + int reg; >> + unsigned channel_mask; >> + unsigned const_block; >> + unsigned const_offset; >> + } *gather_table; >> }; > > Below in brw_shader.h you do: > > struct gather_table { > int reg; > unsigned channel_mask; > unsigned const_block; > unsigned const_offset; > }; > gather_table *ubo_gather_table; > > Why not here?
I guess you're right. Yep, I can probably re-use the same definition in the next version. Thanks! > >> >> /* Data about a particular attempt to compile a program. Note that >> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c >> b/src/mesa/drivers/dri/i965/brw_gs.c >> index bea90d8..97658d5 100644 >> --- a/src/mesa/drivers/dri/i965/brw_gs.c >> +++ b/src/mesa/drivers/dri/i965/brw_gs.c >> @@ -70,6 +70,10 @@ brw_compile_gs_prog(struct brw_context *brw, >> c.prog_data.base.base.pull_param = >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> c.prog_data.base.base.nr_params = param_count; >> + c.prog_data.base.base.nr_gather_table = 0; >> + c.prog_data.base.base.gather_table = >> + rzalloc_size(NULL, sizeof(*c.prog_data.base.base.gather_table) * >> + (c.prog_data.base.base.nr_params + >> c.prog_data.base.base.nr_ubo_params)); > > Wrap this line. > >> >> if (brw->gen >= 7) { >> if (gp->program.OutputType == GL_POINTS) { >> diff --git a/src/mesa/drivers/dri/i965/brw_program.c >> b/src/mesa/drivers/dri/i965/brw_program.c >> index 81a0c19..f27c799 100644 >> --- a/src/mesa/drivers/dri/i965/brw_program.c >> +++ b/src/mesa/drivers/dri/i965/brw_program.c >> @@ -573,6 +573,10 @@ brw_stage_prog_data_compare(const struct >> brw_stage_prog_data *a, >> if (memcmp(a->pull_param, b->pull_param, a->nr_pull_params * sizeof(void >> *))) >> return false; >> >> + if (memcmp(a->gather_table, b->gather_table, >> + a->nr_gather_table * sizeof(*a->gather_table))) >> + return false; >> + >> return true; >> } >> >> @@ -583,6 +587,7 @@ brw_stage_prog_data_free(const void *p) >> >> ralloc_free(prog_data->param); >> ralloc_free(prog_data->pull_param); >> + ralloc_free(prog_data->gather_table); >> } >> >> void >> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp >> b/src/mesa/drivers/dri/i965/brw_shader.cpp >> index 0d6ac0c..8769f67 100644 >> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp >> @@ -739,11 +739,13 @@ backend_visitor::backend_visitor(struct brw_context >> *brw, >> prog(prog), >> stage_prog_data(stage_prog_data), >> cfg(NULL), >> - stage(stage) >> + stage(stage), >> + ubo_gather_table(NULL) >> { >> debug_enabled = INTEL_DEBUG & intel_debug_flag_for_shader_stage(stage); >> stage_name = _mesa_shader_stage_to_string(stage); >> stage_abbrev = _mesa_shader_stage_to_abbrev(stage); >> + this->nr_ubo_gather_table = 0; > > Any particular reason not to do this in the initializer along with the other > members? Nothing really. Yeah, it makes sense to put this in the initializer. > >> } >> >> bool >> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h >> b/src/mesa/drivers/dri/i965/brw_shader.h >> index 8a3263e..db0018f 100644 >> --- a/src/mesa/drivers/dri/i965/brw_shader.h >> +++ b/src/mesa/drivers/dri/i965/brw_shader.h >> @@ -204,6 +204,17 @@ public: >> void assign_common_binding_table_offsets(uint32_t >> next_binding_table_offset); >> >> virtual void invalidate_live_intervals() = 0; >> + >> + /** Gather table entries for UBOs */ >> + unsigned nr_ubo_gather_table; >> + >> + struct gather_table { >> + int reg; >> + unsigned channel_mask; >> + unsigned const_block; >> + unsigned const_offset; >> + }; >> + gather_table *ubo_gather_table; >> }; >> >> uint32_t brw_texture_offset(struct gl_context *ctx, int *offsets, >> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c >> b/src/mesa/drivers/dri/i965/brw_vs.c >> index dabff43..52333c9 100644 >> --- a/src/mesa/drivers/dri/i965/brw_vs.c >> +++ b/src/mesa/drivers/dri/i965/brw_vs.c >> @@ -243,6 +243,11 @@ brw_compile_vs_prog(struct brw_context *brw, >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> stage_prog_data->nr_params = param_count; >> >> + stage_prog_data->nr_gather_table = 0; >> + stage_prog_data->gather_table = rzalloc_size(NULL, >> sizeof(*stage_prog_data->gather_table) * > > Wrap this line. > >> + (stage_prog_data->nr_params >> + >> + >> stage_prog_data->nr_ubo_params)); >> + >> GLbitfield64 outputs_written = vp->program.Base.OutputsWritten; >> prog_data.inputs_read = vp->program.Base.InputsRead; >> >> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c >> b/src/mesa/drivers/dri/i965/brw_wm.c >> index 308eebe..13a64d8 100644 >> --- a/src/mesa/drivers/dri/i965/brw_wm.c >> +++ b/src/mesa/drivers/dri/i965/brw_wm.c >> @@ -205,6 +205,11 @@ brw_compile_wm_prog(struct brw_context *brw, >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> prog_data.base.nr_params = param_count; >> >> + prog_data.base.nr_gather_table = 0; >> + prog_data.base.gather_table = rzalloc_size(NULL, >> sizeof(*prog_data.base.gather_table) * > > Wrap this line. > >> + (prog_data.base.nr_params + >> + >> prog_data.base.nr_ubo_params)); >> + >> prog_data.barycentric_interp_modes = >> brw_compute_barycentric_interp_modes(brw, key->flat_shade, >> key->persample_shading, >> -- >> 1.9.1 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev