[Mesa-dev] [PATCH 9/9] tgsi: add ArrayID documentation

Mon Mar 18 04:29:02 PDT 2013

Am 17.03.2013 20:59, schrieb Christoph Bumiller:
> On 17.03.2013 18:04, Christoph Bumiller wrote:
>> On 17.03.2013 16:30, Christian König wrote:
>>> Am 15.03.2013 18:58, schrieb Christoph Bumiller:
>>>> On 15.03.2013 13:08, Christian König wrote:
>>>>> Am 14.03.2013 15:53, schrieb Christoph Bumiller:
>>>>>> On 14.03.2013 15:20, Christian König wrote:
>>>>>>> From: Christian König <christian.koenig at amd.com>
>>>>>>>
>>>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>>>> ---
>>>>>>>     src/gallium/docs/source/tgsi.rst |   16 ++++++++++++++++
>>>>>>>     1 file changed, 16 insertions(+)
>>>>>>>
>>>>>>> diff --git a/src/gallium/docs/source/tgsi.rst
>>>>>>> b/src/gallium/docs/source/tgsi.rst
>>>>>>> index d9a7fe9..27fe039 100644
>>>>>>> --- a/src/gallium/docs/source/tgsi.rst
>>>>>>> +++ b/src/gallium/docs/source/tgsi.rst
>>>>>>> @@ -1833,6 +1833,22 @@ If Interpolate flag is set to 1, a
>>>>>>> Declaration Interpolate token follows.
>>>>>>>       If file is TGSI_FILE_RESOURCE, a Declaration Resource token
>>>>>>> follows.
>>>>>>>     +If Array flag is set to 1, a Declaration Array token follows.
>>>>>>> +
>>>>>>> +Array Declaration
>>>>>>> +^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>> +
>>>>>>> +Declarations can optional have an ArrayID attribute which can be
>>>>>>> referred by
>>>>>>> +indirect addressing operands. An ArrayID of zero is reserved and
>>>>>>> treaded as
>>>>>>> +if no ArrayID is specified.
>>>>>>> +
>>>>>>> +If an indirect addressing operand refers to an specific declaration
>>>>>>> by using
>>>>>> s/an/a
>>>>> Thx, fixed.
>>>>>
>>>>>>> +an ArrayID only the registers in this declaration are guaranteed
>>>>>>> to be
>>>>>>> +accessed, accessing any register outside this declaration results
>>>>>>> in undefined
>>>>>>> +behavior.
>>>>>> + Note that the effective index is zero-based and not relative to the
>>>>>> specified declaration. XXX: Is it ? Should it be ?
>>>>> Yes for compatibility reasons, otherwise we would need to change all
>>>>> drivers at once.
>>>>>
>>>>>>> +
>>>>>>> +If no ArrayID is specified with an indirect addressing operand the
>>>>>>> whole
>>>>>>> +register file might be accessed by this operand.
>>>>>>>     
>>>>>> + A practice which is strongly discouraged. Don't do this if you have
>>>>>> more than 1 declaration for the file in question ! It will prevent
>>>>>> packing of scalar/vec2 arrays and effective memory alias analysis.
>>>>> A bit shortened, but in general added the remark.
>>>>>
>>>>>> Packing ? Yes !
>>>>>> We can pack arrays if they're declared as e.g.
>>>>>> TEMP[0-3].xyzw
>>>>>> TEMP[4-31].x
>>>>>>
>>>>>> And the caches will be very very thankful that we don't just access
>>>>>> every 4th element of our 4 times larger than it needs to be buffer !!!
>>>>>>
>>>>>> And if your card can't do that, pleeease be nice and still make it
>>>>>> possible for other drivers. :o3
>>>>> It is probably possible with the new information to do so, but not
>>>>> priority for me cause I primary need it for our LLVM backend.
>>>>>
>>>> At some point you'll be able to make use of the info in your backend,
>>>> too, and then you'll regret having to refamiliarize with this code just
>>>> because you didn't add the extra (estimated) 2 lines to set the
>>>> UsageMask.
>>> I think you misunderstood me here, you don't need the UsageMask to
>>> generate those informations. It is possible by just scanning the
>>> shader to figure out which channels are used and which aren't.
>>>
>> For temporaries that may be true ... and inputs/outputs are always vec4
>> sized to guarantee linkage, packing for GENERIC ones is handled at the
>> mesa level.
>>
>>> Additional to that I'm not convinced that using the UsageMask for this
>>> is 100% correct, to me it looks more like UsageMask is something we
>>> need for outputs to distinct between not writing to an output channel
>>> (and so still having the default) and not having an output channel at
>>> all.
>>>
>> Actually, for gl_ClipDistance[] we use the UsageMask to specify if the
>> clip distance was declared in the source (and thus should be enabled)
>> instead of whether it's been written or not.
>>
>> I wanted to be able to distinguish between
>> float gl_ClipDistance[8] or
>> vec4 mesa_ClipDistance[2] with the UsageMask but I guess
>>
>> OUT[0..1].x, CLIPDIST might just as well mean that gl_ClipDistance[0 and
>> 4] are being used ...
>>
>> Hm, we'll need a cap for that anway to tell st if it should lower
>> ClipDistance to vec4s or not, and just assume that TGSI corresponds to
>> what the cap says.
>> And since this is the only case for IN/OUT where the driver's backend
>> has to decide whether to pack or not ... ok, I'll just infer array width
>> myself, too, and you can ignore the UsageMask.
>>
>>>> Also, NAK from me until array access/declarations for the other files
>>>> follows suit.
>>>> Sorry for being so ... pesky, but I'd really like this change to be 100%
>>>> complete. Come on, doesn't it nag on your conscience if this is left to
>>>> remain only a few smalls steps from perfection ?
>>> Declaring and accessing arrays for inputs/outputs are not so much of a
>>> problem, figuring out how to get this information to glsl_to_tgsi is
>>> the real problem. For temporaries changing the glsl_to_tgsi pass is
>>> pretty much sufficient, but for inputs and outputs you need to dig
>>> into the mesa state tracker, and I definitely don't intend to do so.
>>>
>> Fine, then I'll have a look at that myself. * flaming eyes *
> Ok, had a look, had enough. It hurts. At least if you have never touched
> glsl-to-tgsi and go about it expecting it to be easy and straightforward.
> HURTS!

Yes, that was also my initial reaction, and is also the reason why I 
have so many concerns doing more than absolutely necessary.

I'm pretty sure that I've considered everything regarding temporaries 
and array support, but I really don't understand much else.

Christian.

>
>>> Christian.
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>