[Mesa-dev] [PATCH 2/7] i965/fs: Save push constant location information.

[Kenneth Graunke]

On 03/13/2014 05:55 PM, Pohjolainen, Topi wrote:
> On Tue, Mar 11, 2014 at 11:48:51PM -0700, Kenneth Graunke wrote:
>> Previously, both move_uniform_array_access_to_pull_constants() and
>> setup_pull_constants() maintained stack-local arrays with this
>> information.  Storing this information will allow it to be used from
>> multiple functions, allowing us to split and move code around.
>>
>> We'll also eventually want to pass pull constant location information
>> to the SIMD16 compile.  Saving this information will help us do that.
>>
>> Unfortunately, the two functions *cannot* share the contents of the
>> array just yet.  remove_dead_constants() renumbers all the UNIFORM
>> registers to be contiguous starting at zero, so the two functions
>> talk about uniforms using different names.  We can't even remap them,
>> since move_uniform_array_access_to_pull_constants() deletes UNIFORM
>> registers that are only accessed with reladdr, so remove_dead_constants
>> can't even see them.
>>
>> This situation will improve in the next few patches.
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp         | 7 +++++--
>>  src/mesa/drivers/dri/i965/brw_fs.h           | 6 ++++++
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 1 +
>>  3 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index c24d2f8..8faf401 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -1827,7 +1827,7 @@ fs_visitor::remove_dead_constants()
>>  void
>>  fs_visitor::move_uniform_array_access_to_pull_constants()
>>  {
>> -   int pull_constant_loc[uniforms];
>> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
>>  
>>     for (unsigned int i = 0; i < uniforms; i++) {
>>        pull_constant_loc[i] = -1;
>> @@ -1884,6 +1884,9 @@ fs_visitor::move_uniform_array_access_to_pull_constants()
>>        }
>>     }
>>     invalidate_live_intervals();
>> +
>> +   ralloc_free(pull_constant_loc);
>> +   pull_constant_loc = NULL;
>>  }
>>  
>>  /**
>> @@ -1909,7 +1912,7 @@ fs_visitor::setup_pull_constants()
>>      */
>>     unsigned int pull_uniform_base = max_uniform_components;
>>  
>> -   int pull_constant_loc[uniforms];
>> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
>>     for (unsigned int i = 0; i < uniforms; i++) {
>>        if (i < pull_uniform_base) {
>>           pull_constant_loc[i] = -1;
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 86c95df..abfdb10 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -509,6 +509,12 @@ public:
>>     /** Number of uniform variable components visited. */
>>     unsigned uniforms;
>>  
>> +   /**
>> +    * Array mapping UNIFORM register numbers to the pull parameter index,
>> +    * or -1 if this uniform register isn't being uploaded as a pull constant.
>> +    */
>> +   int *pull_constant_loc;
>> +
>>     /* This is the map from UNIFORM hw_reg + reg_offset as generated by
>>      * the visitor to the packed uniform number after
>>      * remove_dead_constants() that represents the actual uploaded
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index 90272eb..404af30 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -2971,6 +2971,7 @@ fs_visitor::fs_visitor(struct brw_context *brw,
>>     this->regs_live_at_ip = NULL;
>>  
>>     this->uniforms = 0;
>> +   this->pull_constant_loc = NULL;
> 
> Apart from 'dispatch_width' all the other members are initialised here in the
> body of the constructor, so it is consistent to have this here also. I'm just
> thinking if we would like to start using more the initialization list.

I don't have a strong preference.  I feel like it's mostly a stylistic
decision, and one I'm happy to leave up to the author.

--Ken

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140313/fde92d13/attachment.pgp>