[Mesa-dev] [PATCH 7/7] tgsi: use separate structure for indirect address

[Christian König]

Am 11.03.2013 22:03, schrieb Jose Fonseca:
>
> ----- Original Message -----
>> Am 11.03.2013 15:52, schrieb Jose Fonseca:
>>> Christian,
>>>
>>> I didn't comment on the previous threads, but the approach mentioned in
>>> http://lists.freedesktop.org/archives/mesa-dev/2012-November/030476.html
>>> seems sensible to me.
>>>
>>> I think after the first round we should have this in a branch to allow
>>> drivers to catch up with the interface change. Or is it possible for
>>> drivers to opt-in via a cap?
>> Not the drivers are in question of changing, the state trackers are. If
>> the drivers just ignore those additional informations nothing should
>> change for them.
> I think that drivers like llvmpipe will choke on declarations like
>
>    DCL TEMP[1][0..7]
>
> So if the state trackers start emitting these I think we'll nee

nee? Sounds like there is something missing in the mail here.

As long as llvmpipe uses the standard TGSI parse function to read the 
tokens I don't see where we will get any problems with that. Maybe 
except that a newer version of mesa dumps the TGSI and an older version 
of mesa tries to read it, but that problem would probably come up with 
any solution.

>
>> For the state trackers my current approach also doesn't need them to
>> change, currently the semantics is as following:
>>
>> If Declaration==0 then we fall back to the "old" behavior, e.g. the
>> whole register file is indirectly addressed.
>> Else the state tracker (currently only glsl_to_tgsi) provided the
>> necessary information in the Declaration field to only indirect address
>> a certain part of the register file.
> Yes, I like that. I think we could eventually be strict and forbid indirect declaration==0, but it's nice not to have to rush.

Ok then I'm going to complete the patchset, document everything and send 
out the resulting patches for a second round of reviews.

>>> A few more remarks inline.
>>>
>>> ----- Original Message -----
>>>> From: Christian König <christian.koenig at amd.com>
>>>>
>>>> To further improve the optimization of source and destination
>>>> indirect addressing we need the ability to store a reference
>>>> to the declaration of the addressed operands.
>>> Just to be perfectly clear, declaration number does not refer to the n-th
>>> TEMP declaration, but declaration of TEMP[n], right?
>> No, currently it indeed refers to the n-th TEMP declaration. But I'm
>> still fighting with myself weather or not that's a good idea.
> I think that using n-th TEMP[?] declaration instead of declaration of TEMP[n] it might be a bad precedent.

I'm convinced now that adding an ArrayID to the declaration and then 
referring to that ID sounds like the best solution for now.

Thanks,
Christian.

>>> That is, this
>>>
>>>     DCL TEMP[1][0..70]
>>>     DCL TEMP[2][0..7]
>>>     MOV OUT[1], TEMP[1][ADDR[0].x]
>>>
>>> and this
>>>
>>>     DCL TEMP[2][0..7]
>>>     DCL TEMP[1][0..70]
>>>     MOV OUT[1], TEMP[1][ADDR[0].x]
>>>
>>> are equivalent, right?
>>>
>>> If so, I wonder if there is a name more descriptive than "Declaration"
>>> here. Maybe "Range", or "IndexableRange"?
>> Correct, yes. As said above, currently "Declaration" refers to the n-th
>> declaration but I can easily add an "Indirect" flag to tgsi_declaration
> I think I'd prefer something along these lines.
>
>> (there are still 6 bits of padding in it) and have an "IndirectRangeID"
>> (or ArrayID, ArrayName, whatever, make your choice) token following the
>> declaration.
> "Array*" sounds great to me.
>
> Jose
>