[Mesa-dev] [RFC] Solving the TGSI indirect addressing optimization problem

[Christian König]

Am 12.03.2013 02:48, schrieb Marek Olšák:
> On Mon, Mar 11, 2013 at 1:44 PM, Christian König
> <deathsimple at vodafone.de> wrote:
>> Hi everybody,
>>
>> this problem has been open for quite some time now, with a bunch of different
>> opinions and sometimes even patches floating on the list.
>>
>> The solutions proposed or implemented so far all more or less incomplete, so
>> this approach was designed in mind with both completeness and compatibility
>> with existing code.
>>
>> Over all it's just an implementation of what Tom Stellard named solution #4 in
>> this eMail thread: http://lists.freedesktop.org/archives/mesa-dev/2013-January/033264.html
> Hi Christian,
>
> this is definitely not the solution #4. According to the TGSI dump
> Christoph posted, it looks more like #3.

Well, for me the main difference between proposal #3 and #4 is that #3 
tries to identify the declaration to use with the supplied "offset", 
while #4 uses a completely distinct identifier for that.

> The solution #4 completely changes the temporary file such that it
> becomes two-dimensional with the first index being a literal and the
> second index being either a literal or ADDR[literal], and it would
> always be like that regardless of whether drivers support that or not.
> One-dimensional indexing of TEMP is not allowed. For backward
> compatibility, the drivers that do not support it would only get a
> single array declaration TEMP[0][0..n] and TEMP[0][...] would be
> everywhere in the code.

Ok, then I misunderstood you a bit, but I don't think the difference is 
so much.

What I'm proposing is that we have an optional "ArrayID" attached to 
each declaration and refer to this "ArrayID" in the indirect addressing 
operand. To sum it up declarations should look something like this:

DCL TEMP[0..3]        // normal registers
DCL TEMP[1][4..11]    // indirectly accessed array
DCL TEMP[2][12..15]    // another indirectly accessed array
DCL TEMP[16..17] LOCAL    // local registers

While an indirect operand might look like this:

MOV TEMP[16], TEMP[1][ADDR[0].x-13]

On the pro side for this approach is that it is compatible with all the 
existing state trackers and driver, and we don't need to generate 
different code depending on weather or not the driver supports this.

> I don't know much about TGSI internals, so I can't review this. I'd
> just like to say that TGSI dumps should make sense (2D indexing should
> be only allowed with 2D declarations) and tgsi_text_translate should
> be able to do the reverse - convert the dumps back to TGSI tokens.

Completely agree with that, and beside writing documentation testing 
this is still one of the todos with this patchset.

I have to admit that your approach looks a bit cleaner from the high 
above view. The problem with it is that it requires this additional 2D 
index on every operand, and we just don't have enough bits left for 
this. Even with my approach I need to make room for this ArrayID in the 
indirect addressing operand token, and this additional token is only 
there if the operand uses indirect adressing.

Do you think we can live with my approach or is there any major downside 
I currently don't see?

Thanks for the clarification,
Christian.