[Mesa-dev] [PATCH 1/2] i965: Allocate vec4_visitor's uniform_size and uniform_vector_size arrays dynamically.

[Francisco Jerez]

Petri Latvala <petri.latvala at intel.com> writes:

> On 11/22/2013 08:05 PM, Francisco Jerez wrote:
>> Petri Latvala<petri.latvala at intel.com>  writes:
>>
>>> [...]
>>> @@ -325,8 +326,9 @@ public:
>>>       */
>>>      dst_reg output_reg[BRW_VARYING_SLOT_COUNT];
>>>      const char *output_reg_annotation[BRW_VARYING_SLOT_COUNT];
>>> -   int uniform_size[MAX_UNIFORMS];
>>> -   int uniform_vector_size[MAX_UNIFORMS];
>>> +   unsigned uniform_param_count;
>>> +   int* uniform_size;
>>> +   int* uniform_vector_size;
>>>      int uniforms;
>>>
>> All the code around you uses the 'type *identifier' convention, please
>> keep it consistent.
>
> *slap on forehead* note to self, disable muscle memory. Fixing that.
>
>>>      src_reg shader_start_time;
>>> [...]
>>> @@ -556,7 +558,7 @@ brw_gs_emit(struct brw_context *brw,
>>>      if (likely(!(INTEL_DEBUG&  DEBUG_NO_DUAL_OBJECT_GS))) {
>>>         c->prog_data.dual_instanced_dispatch = false;
>>>
>>> -      vec4_gs_visitor v(brw, c, prog, shader, mem_ctx, true /* no_spills */);
>>> +      vec4_gs_visitor v(brw, c, prog, shader, mem_ctx, true /* no_spills */, param_count);
>> Another possibility would be to set 'c.prog_data.base.nr_params =
>> param_count' here and in brw_vs_emit(), so you'd have the same value
>> available from the constructor of vec4_visitor without all the parameter
>> changes.
>
> When trying this change, I began to wonder if I'm using the right values 
> at all. What is that nr_params supposed to be for code elsewhere? 
> vec4_visitor::setup_uniforms() sets it to this->uniforms * 4, but 
> param_count that I passed from do_vs_prog() to brw_vs_emit() is 
> num_uniform_components * 4, yielding (close to) this->uniforms * 4 * 4. 

Oh...  Apparently do_vs_prog() is overallocating memory.  And so is
do_gs_prog()...  That seems like a bug.

> Close to means that my test program gives param_count being 92, and 
> this->uniforms * 4 in setup_uniforms() is 24.
>
> Is vec4_visitor::setup_uniforms() supposed to be the authoritative 
> source of the uniform counts or is that nr_params assignment just for 
> the pre-gen6 hack in the same function? Is nr_params supposed to be the 
> number of uniform components or uniforms?

It should be the number of uniform components (in units of 32-bit
floats).

> To my understanding param_count in do_vs_prog() is the number of
> uniform components required to hold all uniforms with a vec4 for each
> one (float or otherwise), are the floats packed to vec4s anywhere?
>
Yes, see vec4_visitor::pack_uniform_registers().

>
> -- 
> Petri Latvala
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131125/88a02853/attachment.pgp>