Re: [Mesa-dev] [PATCH 1/7] gallium: add CONSTBUF type to tgsi_file_type
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 Scheideggerwrote: > 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
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
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ähnlewrote: 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
Am 23.08.2017 um 15:49 schrieb Ilia Mirkin: > On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnlewrote: >> 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
On 23.08.2017 15:49, Ilia Mirkin wrote: On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnlewrote: 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
On Wed, Aug 23, 2017 at 9:20 AM, Nicolai Hähnlewrote: > 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
On 22.08.2017 16:56, Ilia Mirkin wrote: On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheideggerwrote: 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
On 23/08/17 09:42, Ilia Mirkin wrote: On Tue, Aug 22, 2017 at 7:41 PM, Timothy Arceriwrote: 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
On Tue, Aug 22, 2017 at 7:41 PM, Timothy Arceriwrote: > > > 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
On 23/08/17 09:28, Ilia Mirkin wrote: On Tue, Aug 22, 2017 at 7:25 PM, Timothy Arceriwrote: 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
On Tue, Aug 22, 2017 at 7:25 PM, Timothy Arceriwrote: > > > 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
On 23/08/17 09:08, Ilia Mirkin wrote: On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceriwrote: 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
On Tue, Aug 22, 2017 at 6:55 PM, Timothy Arceriwrote: > > > 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
On 23/08/17 00:56, Ilia Mirkin wrote: On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheideggerwrote: 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
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
On Tue, Aug 22, 2017 at 10:51 AM, Roland Scheideggerwrote: > 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
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
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