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

[Marek Olšák]

I'd like CONSTBUF to stay. I think CONST is already too overloaded.

If we start treating CONST[0] and CONST[0][0] differently, things are
just going to get more ugly. If the second dimension is unspecified,
it's implied to be 0. TGSI doesn't allow specifying the second
dimension index and omit the first dimenstion, which means it can't
say that CONST[0] is constbuf 0 without changing existing conventions.

Marek


On Wed, Aug 23, 2017 at 5:15 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> 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.
>>>>
>>>
>>
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev