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

[Marek Olšák]

On Tue, Mar 12, 2013 at 10:31 AM, Christian König
<deathsimple at vodafone.de> wrote:
> 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.

In that case, it would be better to avoid using the operator [] and
use something else, because it has nothing to do with indexing.

>
>
>> 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?

I'm okay with your approach. Although it doesn't appear to be very
clean, it gives the same amount of information to drivers.

Marek