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

[Roland Scheidegger]

Am 23.08.2017 um 17:03 schrieb 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.
I suppose it wouldn't be too difficult to make them just accept this
(basically ignoring the buffer index).
But anyway, I don't know if it's worth the hassle, I just brought it up
because if it's a problem going forward, it should be possible to change
it. (Albeit we definitely have code relying on these 1d constants too...)

Roland


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