[Mesa-dev] [PATCH] glsl_to_tgsi: indirect array information

[Christoph Bumiller]

On 23.01.2013 01:47, Vadim Girlin wrote:
> On 01/23/2013 04:07 AM, Dave Airlie wrote:
>> On Wed, Jan 23, 2013 at 9:44 AM, Vadim Girlin <vadimgirlin at gmail.com>
>> wrote:
>>> On 01/22/2013 10:59 PM, Christoph Bumiller wrote:
>>>>
>>>> On 21.01.2013 21:10, Vadim Girlin wrote:
>>>>>
>>>>> Provide the information about indirectly addressable arrays (ranges of
>>>>> temps) in
>>>>> the shader to the drivers. TGSI representation itself isn't modified,
>>>>> array
>>>>> information is passed as an additional data in the
>>>>> pipe_shader_state, so
>>>>> the
>>>>> drivers can use it as a hint for optimization.
>>>>> ---
>>>>>
>>>>> It's far from being an ideal solution, but I saw the discussions about
>>>>> that
>>>>> problem starting from 2009 IIRC, and we still have no solution
>>>>> (neither
>>>>> good
>>>>> nor bad) despite the years passed. I hope we can use this not very
>>>>> intrusive
>>>>> approach until we get something better.
>>>>>
>>>>
>>>> I'd rather not have any hacks in the interface, let alone ones that
>>>> solve the problem only partially (you still won't know which array is
>>>> accessed by a particular instruction, which is important for
>>>> optimization and essential in some cases for making INPUT/OUTPUT arrays
>>>> work), and not just because it reduces the pressure on people to
>>>> implement a proper solution.
>>>>
>>>> With this, you just get to know which range of TEMPs are indirectly
>>>> addressed and which ones are not, and you can do the same by simply
>>>> creating multiple declarations of TEMPs, one for each array, and adding
>>>> a single bit of info to tgsi_declaration (which has 7 bits of padding
>>>> anyway, so ample space), which is a lot less ugly, and doesn't suffer
>>>> from an arbitrary limit, and doesn't require any modification of
>>>> drivers
>>>> either.
>>>>
>>>
>>> Array accessed by any indirect operand can be identified by the
>>> immediate
>>> offset, e.g. TEMP[ADDR[0].x+1] implies the array starting from 1,
>>> thus we
>>> can find it's entry in the information provided by this patch to get the
>>> addressable range for every indirect operand. If I'm not missing
>>> something,
>>> glsl_to_tgsi accumulates all other parts of the offset in the address
>>> register before the indirect access. If I'm wrong, we can fix it to
>>> ensure
>>> such behavior.
>>>
>>> I'll be perfectly OK with any other solution, as long as it's a really
>>> working (already implemented) solution that I can use today, not just
>>> some
>>> abstract ideas in the discussions. This patch isn't perfect and can be
>>> improved, but it already works for me. I'll be very happy to use any
>>> other
>>> solution from you or anyone else.
>>>
>>
>> It doesn't work like that, this patch isn't what I'd like either, it
>> hacks around the interface,
>>
>> If what calim suggests works, then someone should be implementing that.
> 
> I agree that someone probably should be implementing that if it works.
> 
>>
>> i prefer the idea that the TGSI shader is self contained.
>>
> 
> I'm not sure what exactly do you mean. We have stream output info as a
> separate structure in the pipe_shader_state, despite the fact that it
> affects shader behavior - it changes produced outputs, we are adding
> instructions for that. Array info in this patch doesn't affect neither
> shader behavior nor the results produced by the shader. The shader will
> work and produce the same results with or without that information. Will
> it work as expected if we'll remove stream output info? So how does this
> patch affect self-containedness more than separate structure for the
> stream output info that perfectly lives in the master.
> 

This isn't really comparable, stream output is separate from the shader,
it doesn't provide any additional information about it (i.e. anything
that is lost in the process of transforming GLSL source into TGSI, like
array access), and its influence on the shader code itself is specific
to radeon.
E.g. for NV cards, you could provide it as a separate state object. It
being tied in with shader state is a concession to existing APIs.

> Vadim
> 
>> Dave.
>>
>