[Mesa-dev] [PATCH 1/7] gallium: add CONSTBUF type to tgsi_file_type

[Nicolai Hähnle]

On 23.08.2017 16:03, Roland Scheidegger wrote:
> Am 23.08.2017 um 15:49 schrieb Ilia Mirkin:
>> On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> On 22.08.2017 16:56, Ilia Mirkin wrote:
>>>>
>>>> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger <sroland at vmware.com>
>>>> wrote:
>>>>>
>>>>> I am probably missing something here, but why do you need a new register
>>>>> file? Since you couldn't use LOAD with TGSI_FILE_CONSTANT before, can't
>>>>> you just allow LOAD with TGSI_FILE_CONSTANT and achieve the same thing?
>>>>> Or do you need to know how it's going to be accessed in advance?
>>>>
>>>>
>>>> With bindless, LOAD can take a CONST I believe [which contains the
>>>> value of the bindless id]. I think it's nice to keep those concepts
>>>> separate... having CONST sometimes mean the value and other times mean
>>>> the address is a bit weird. This way CONSTBUF[0] is the address of the
>>>> 0th constbuf.
>>>
>>>
>>> I'm still not quite convinced. The levels of indirection should clarify the
>>> meaning, shouldn't they?
>>>
>>> You get
>>>
>>>    LOAD dst, CONST[0][0], IMM[0]
>>>
>>> when loading from offset IMM[0] of a bindless buffer whose handle is at the
>>> beginning of the buffer CONST[0].
>>>
>>> You get
>>>
>>>    LOAD dst, CONST[0], IMM[0]
>>>
>>> when loading from offset IMM[0] of non-bindless buffer 0.
>>>
>>> Is there ever really a situation where the two could be confused?
>>
>> I always considered CONST[0] == CONST[0][0]. Technically they're not,
>> since once has the second dimension in the TGSI encoding while the
>> other doesn't. But practically,
>>
>> MOV TEMP[0], CONST[0]
>>
>> and
>>
>> MOV TEMP[0], CONST[0][0]
>>
>> are in every way identical. Currently st/mesa will just use CONST[0]
>> everywhere, never adding the 2nd dimension.
> Maybe it would be worth the effort to fix this?

Would be nice. One thing that makes this a bit awkward is that older 
drivers just don't support two-dimensional CONST at all -- see 
PIPE_SHADER_CAP_MAX_CONST_BUFFERS. Giving them a shader that loads 
CONST[0][n] is going to fail.

Basically, changing this is a backward-compatible change to state 
trackers, which would have to promise not to produce one-dimensional 
CONST for the usual, vec4-based constant fetching.

On the other hand, maybe we're over-complicating this. The only 
instruction that is really affected is LOAD. And for LOAD, there 
shouldn't be a compatibility problem. Hmm...

Cheers,
Nicolai

> 
> Roland
> 
> 
>   As such, I don't think we
>> should start having behavioural differences for those on some
>> instructions.
>>
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.