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.koe...@amd.com> >>>>>> >>>>>> Signed-off-by: Christian König <christian.koe...@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! >> Christian. > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
