[Mesa-dev] [PATCH 11/27] i965: Store gather table information in the program data
Abdiel Janulgue
abdiel.janulgue at linux.intel.com
Fri May 8 05:44:35 PDT 2015
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.janulgue at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list