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

2017-08-29 Thread 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  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 
 wrote:
> On 22.08.2017 16:56, Ilia Mirkin wrote:
>>
>> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger
>> 
>> 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-23 Thread 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 
>>> wrote:
 On 22.08.2017 16:56, Ilia Mirkin wrote:
>
> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger
> 
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-23 Thread 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  wrote:

On 22.08.2017 16:56, Ilia Mirkin wrote:


On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger 
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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-23 Thread Roland Scheidegger
Am 23.08.2017 um 15:49 schrieb Ilia Mirkin:
> On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnle  wrote:
>> On 22.08.2017 16:56, Ilia Mirkin wrote:
>>>
>>> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger 
>>> 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?

Roland


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

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-23 Thread Nicolai Hähnle

On 23.08.2017 15:49, Ilia Mirkin wrote:

On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnle  wrote:

On 22.08.2017 16:56, Ilia Mirkin wrote:


On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger 
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. As such, I don't think we
should start having behavioural differences for those on some
instructions.


Oh, you're right, CONST[n] and CONST[0][n] are treated the same. That's 
pretty inconsistent and unfortunate :/


I suppose the CONSTBUF thing is fine, then.

Cheers,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-23 Thread Ilia Mirkin
On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnle  wrote:
> On 22.08.2017 16:56, Ilia Mirkin wrote:
>>
>> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger 
>> 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. As such, I don't think we
should start having behavioural differences for those on some
instructions.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-23 Thread Nicolai Hähnle

On 22.08.2017 16:56, Ilia Mirkin wrote:

On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger  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?

Cheers,
Nicolai




   -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Timothy Arceri

On 23/08/17 09:42, Ilia Mirkin wrote:

On Tue, Aug 22, 2017 at 7:41 PM, Timothy Arceri  wrote:



On 23/08/17 09:28, Ilia Mirkin wrote:


On Tue, Aug 22, 2017 at 7:25 PM, Timothy Arceri 
wrote:




On 23/08/17 09:08, Ilia Mirkin wrote:



On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceri 
wrote:





On 23/08/17 00:56, Ilia Mirkin wrote:




On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger

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.





Yeah. I think we also may need another type for bindless as I'm
planning
to
use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD
for
supporting packed uniforms rather than padding everything to vec4.




Shouldn't be necessary... we can think of CONST (and TEMP and IMM) as
"value" registers, and MEMORY/IMAGE/BUFFER/CONSTBUF as "address"
registers. If LOAD receives a value, then it's a bindless image
handle, otherwise it should work based on which of the address
registers it receives.




But how do you tell the difference between a bindless image handle and a
non-indirect uniform where the "value" is just the index of the uniform?



Easy - if the first arg is a CONSTBUF[], it's a uniform load. If it's
a value, then it's a bindless image handle. A uniform load becomes

LOAD dst, CONSTBUF[1], IMM[0].x

which would be identical to doing

MOV dst, CONST[1][5] (if IMM[0].x == 5)



I'm talking about using:

CONSTBUF for UBOs

CONSTANT for uniforms

SOMETHINGELSE for bindless images

As far as I can tell we need to differentiate between uniforms and ubos, and
there doesn't seem to be anything else to help with that.


Gallium doesn't differentiate between uniform and UBO. In practice,
st/mesa sticks uniforms in the zero const slot.



Yeah ok, looking at the code again that makes sense. Thanks for clarifying.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Ilia Mirkin
On Tue, Aug 22, 2017 at 7:41 PM, Timothy Arceri  wrote:
>
>
> On 23/08/17 09:28, Ilia Mirkin wrote:
>>
>> On Tue, Aug 22, 2017 at 7:25 PM, Timothy Arceri 
>> wrote:
>>>
>>>
>>>
>>> On 23/08/17 09:08, Ilia Mirkin wrote:


 On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceri 
 wrote:
>
>
>
>
> On 23/08/17 00:56, Ilia Mirkin wrote:
>>
>>
>>
>> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger
>> 
>> 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.
>
>
>
>
> Yeah. I think we also may need another type for bindless as I'm
> planning
> to
> use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD
> for
> supporting packed uniforms rather than padding everything to vec4.



 Shouldn't be necessary... we can think of CONST (and TEMP and IMM) as
 "value" registers, and MEMORY/IMAGE/BUFFER/CONSTBUF as "address"
 registers. If LOAD receives a value, then it's a bindless image
 handle, otherwise it should work based on which of the address
 registers it receives.
>>>
>>>
>>>
>>> But how do you tell the difference between a bindless image handle and a
>>> non-indirect uniform where the "value" is just the index of the uniform?
>>
>>
>> Easy - if the first arg is a CONSTBUF[], it's a uniform load. If it's
>> a value, then it's a bindless image handle. A uniform load becomes
>>
>> LOAD dst, CONSTBUF[1], IMM[0].x
>>
>> which would be identical to doing
>>
>> MOV dst, CONST[1][5] (if IMM[0].x == 5)
>>
>
> I'm talking about using:
>
> CONSTBUF for UBOs
>
> CONSTANT for uniforms
>
> SOMETHINGELSE for bindless images
>
> As far as I can tell we need to differentiate between uniforms and ubos, and
> there doesn't seem to be anything else to help with that.

Gallium doesn't differentiate between uniform and UBO. In practice,
st/mesa sticks uniforms in the zero const slot.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Timothy Arceri



On 23/08/17 09:28, Ilia Mirkin wrote:

On Tue, Aug 22, 2017 at 7:25 PM, Timothy Arceri  wrote:



On 23/08/17 09:08, Ilia Mirkin wrote:


On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceri 
wrote:




On 23/08/17 00:56, Ilia Mirkin wrote:



On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger

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.




Yeah. I think we also may need another type for bindless as I'm planning
to
use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD for
supporting packed uniforms rather than padding everything to vec4.



Shouldn't be necessary... we can think of CONST (and TEMP and IMM) as
"value" registers, and MEMORY/IMAGE/BUFFER/CONSTBUF as "address"
registers. If LOAD receives a value, then it's a bindless image
handle, otherwise it should work based on which of the address
registers it receives.



But how do you tell the difference between a bindless image handle and a
non-indirect uniform where the "value" is just the index of the uniform?


Easy - if the first arg is a CONSTBUF[], it's a uniform load. If it's
a value, then it's a bindless image handle. A uniform load becomes

LOAD dst, CONSTBUF[1], IMM[0].x

which would be identical to doing

MOV dst, CONST[1][5] (if IMM[0].x == 5)



I'm talking about using:

CONSTBUF for UBOs

CONSTANT for uniforms

SOMETHINGELSE for bindless images

As far as I can tell we need to differentiate between uniforms and ubos, 
and there doesn't seem to be anything else to help with that.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Ilia Mirkin
On Tue, Aug 22, 2017 at 7:25 PM, Timothy Arceri  wrote:
>
>
> On 23/08/17 09:08, Ilia Mirkin wrote:
>>
>> On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceri 
>> wrote:
>>>
>>>
>>>
>>> On 23/08/17 00:56, Ilia Mirkin wrote:


 On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger
 
 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.
>>>
>>>
>>>
>>> Yeah. I think we also may need another type for bindless as I'm planning
>>> to
>>> use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD for
>>> supporting packed uniforms rather than padding everything to vec4.
>>
>>
>> Shouldn't be necessary... we can think of CONST (and TEMP and IMM) as
>> "value" registers, and MEMORY/IMAGE/BUFFER/CONSTBUF as "address"
>> registers. If LOAD receives a value, then it's a bindless image
>> handle, otherwise it should work based on which of the address
>> registers it receives.
>
>
> But how do you tell the difference between a bindless image handle and a
> non-indirect uniform where the "value" is just the index of the uniform?

Easy - if the first arg is a CONSTBUF[], it's a uniform load. If it's
a value, then it's a bindless image handle. A uniform load becomes

LOAD dst, CONSTBUF[1], IMM[0].x

which would be identical to doing

MOV dst, CONST[1][5] (if IMM[0].x == 5)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Timothy Arceri



On 23/08/17 09:08, Ilia Mirkin wrote:

On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceri  wrote:



On 23/08/17 00:56, Ilia Mirkin wrote:


On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger 
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.



Yeah. I think we also may need another type for bindless as I'm planning to
use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD for
supporting packed uniforms rather than padding everything to vec4.


Shouldn't be necessary... we can think of CONST (and TEMP and IMM) as
"value" registers, and MEMORY/IMAGE/BUFFER/CONSTBUF as "address"
registers. If LOAD receives a value, then it's a bindless image
handle, otherwise it should work based on which of the address
registers it receives.


But how do you tell the difference between a bindless image handle and a 
non-indirect uniform where the "value" is just the index of the uniform?





   -ilia


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Ilia Mirkin
On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceri  wrote:
>
>
> On 23/08/17 00:56, Ilia Mirkin wrote:
>>
>> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger 
>> 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.
>
>
> Yeah. I think we also may need another type for bindless as I'm planning to
> use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD for
> supporting packed uniforms rather than padding everything to vec4.

Shouldn't be necessary... we can think of CONST (and TEMP and IMM) as
"value" registers, and MEMORY/IMAGE/BUFFER/CONSTBUF as "address"
registers. If LOAD receives a value, then it's a bindless image
handle, otherwise it should work based on which of the address
registers it receives.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Timothy Arceri



On 23/08/17 00:56, Ilia Mirkin wrote:

On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger  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.


Yeah. I think we also may need another type for bindless as I'm planning 
to use TGSI_FILE_CONSTANT for regular uniforms. The plan is to use LOAD 
for supporting packed uniforms rather than padding everything to vec4.





   -ilia


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Roland Scheidegger
Am 22.08.2017 um 16:56 schrieb Ilia Mirkin:
> On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger  
> 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.

Ok, makes sense then.

Roland

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Ilia Mirkin
On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheidegger  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.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Roland Scheidegger
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?

Roland

Am 22.08.2017 um 14:14 schrieb Timothy Arceri:
> This will be use to distinguish between load types when using
> the TGSI_OPCODE_LOAD opcode.
> ---
>  src/gallium/auxiliary/tgsi/tgsi_strings.c  | 1 +
>  src/gallium/include/pipe/p_shader_tokens.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.c 
> b/src/gallium/auxiliary/tgsi/tgsi_strings.c
> index 7ce12d3655..0872db9ce8 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_strings.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_strings.c
> @@ -50,20 +50,21 @@ static const char *tgsi_file_names[] =
> "OUT",
> "TEMP",
> "SAMP",
> "ADDR",
> "IMM",
> "SV",
> "IMAGE",
> "SVIEW",
> "BUFFER",
> "MEMORY",
> +   "CONSTBUF",
>  };
>  
>  const char *tgsi_semantic_names[TGSI_SEMANTIC_COUNT] =
>  {
> "POSITION",
> "COLOR",
> "BCOLOR",
> "FOG",
> "PSIZE",
> "GENERIC",
> diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
> b/src/gallium/include/pipe/p_shader_tokens.h
> index aa0fb3e3b3..f9cb6183ce 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -67,20 +67,21 @@ enum tgsi_file_type {
> TGSI_FILE_OUTPUT,
> TGSI_FILE_TEMPORARY,
> TGSI_FILE_SAMPLER,
> TGSI_FILE_ADDRESS,
> TGSI_FILE_IMMEDIATE,
> TGSI_FILE_SYSTEM_VALUE,
> TGSI_FILE_IMAGE,
> TGSI_FILE_SAMPLER_VIEW,
> TGSI_FILE_BUFFER,
> TGSI_FILE_MEMORY,
> +   TGSI_FILE_CONSTBUF,
> TGSI_FILE_COUNT,  /**< how many TGSI_FILE_ types */
>  };
>  
>  
>  #define TGSI_WRITEMASK_NONE 0x00
>  #define TGSI_WRITEMASK_X0x01
>  #define TGSI_WRITEMASK_Y0x02
>  #define TGSI_WRITEMASK_XY   0x03
>  #define TGSI_WRITEMASK_Z0x04
>  #define TGSI_WRITEMASK_XZ   0x05
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2017-08-22 Thread Timothy Arceri
This will be use to distinguish between load types when using
the TGSI_OPCODE_LOAD opcode.
---
 src/gallium/auxiliary/tgsi/tgsi_strings.c  | 1 +
 src/gallium/include/pipe/p_shader_tokens.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.c 
b/src/gallium/auxiliary/tgsi/tgsi_strings.c
index 7ce12d3655..0872db9ce8 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_strings.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_strings.c
@@ -50,20 +50,21 @@ static const char *tgsi_file_names[] =
"OUT",
"TEMP",
"SAMP",
"ADDR",
"IMM",
"SV",
"IMAGE",
"SVIEW",
"BUFFER",
"MEMORY",
+   "CONSTBUF",
 };
 
 const char *tgsi_semantic_names[TGSI_SEMANTIC_COUNT] =
 {
"POSITION",
"COLOR",
"BCOLOR",
"FOG",
"PSIZE",
"GENERIC",
diff --git a/src/gallium/include/pipe/p_shader_tokens.h 
b/src/gallium/include/pipe/p_shader_tokens.h
index aa0fb3e3b3..f9cb6183ce 100644
--- a/src/gallium/include/pipe/p_shader_tokens.h
+++ b/src/gallium/include/pipe/p_shader_tokens.h
@@ -67,20 +67,21 @@ enum tgsi_file_type {
TGSI_FILE_OUTPUT,
TGSI_FILE_TEMPORARY,
TGSI_FILE_SAMPLER,
TGSI_FILE_ADDRESS,
TGSI_FILE_IMMEDIATE,
TGSI_FILE_SYSTEM_VALUE,
TGSI_FILE_IMAGE,
TGSI_FILE_SAMPLER_VIEW,
TGSI_FILE_BUFFER,
TGSI_FILE_MEMORY,
+   TGSI_FILE_CONSTBUF,
TGSI_FILE_COUNT,  /**< how many TGSI_FILE_ types */
 };
 
 
 #define TGSI_WRITEMASK_NONE 0x00
 #define TGSI_WRITEMASK_X0x01
 #define TGSI_WRITEMASK_Y0x02
 #define TGSI_WRITEMASK_XY   0x03
 #define TGSI_WRITEMASK_Z0x04
 #define TGSI_WRITEMASK_XZ   0x05
-- 
2.13.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev