[Mesa-dev] [PATCH 03/11] i965/vec4: use attribute slots for first non payload GRF

[Alejandro Piñeiro]

On 10/01/17 15:52, Jordan Justen wrote:
> On 2017-01-09 09:10:01, Juan A. Suarez Romero wrote:
>> From: Alejandro Piñeiro <apinheiro at igalia.com>
>>
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> index 5ddbe58..f80162d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -1757,7 +1757,7 @@ vec4_vs_visitor::setup_attributes(int payload_reg)
>>  
>>     lower_attributes_to_hw_regs(attribute_map, false /* interleaved */);
>>  
>> -   return payload_reg + vs_prog_data->nr_attributes;
>> +   return payload_reg + vs_prog_data->nr_attribute_slots;
> This path will be hit by gen >= 8, right? 

No, because gen >= 8 doesn't use vec4, it uses the scalar mode. For gen
>= 8, the equivalent method called is fs_visitor::assign_vs_urb_setup
(at brw_fs.cpp).

> Can you explain why it is
> needed for gen < 8, 

It is needed because the function returns the first GRF that is not part
of the payload. As 64-bit attributes can consume more that one slot, we
can't just add the number of attributes.
> and if appropriate, why it is okay for gen >= 8?

As I mentioned, it doesn't affect gen >= 8

>
> The explanation could be something that you can then add to the commit
> message...

Ok, so how about a commit message like this?:

"i965/vec4: use attribute slots for first non payload GRF

As part of the payload setup, setup_attributes is called with the first
GRF that can be used for the attributes (first ones are used for
uniforms for example) and returns the first GRF that is not part of the
payload. Before this patch, it adds directly the number of attributes.
But as with 64-bit attributes can consume more than one slot, that is
not valid anymore. This patch change the addition to use the number of
slots consumed.

gen >= 8 would not be affected, as they use the scalar mode. For that
case, the vs configuration is done at fs_visitor::assign_vs_urb_setup."