Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
Am 03.02.2018 um 03:12 schrieb Marek Olšák: > On Sat, Feb 3, 2018 at 2:55 AM, Roland Scheidegger wrote: >> Am 03.02.2018 um 00:31 schrieb Marek Olšák: >>> On Sat, Feb 3, 2018 at 12:01 AM, Roland Scheidegger >>> wrote: Am 02.02.2018 um 23:39 schrieb Marek Olšák: > On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger > wrote: >> Am 02.02.2018 um 21:48 schrieb Marek Olšák: >>> Hi, >>> >>> This is the second and hopefully final version of 32-bit pointer >>> support for radeonsi. >>> >>> Constant buffer 0 now has restrictions on which buffers can be set >>> in that slot. >>> >>> I plan to push this when my LLVM patch lands in 6.0 (hopefully it >>> will be accepted there). >>> >>> There will also be a dependency on new libdrm (not included in this >>> series). >>> >>> Please review. >>> >> >> From a api cleanliness point of view, I don't like this much. >> First, you're making the hack case the default and even require it. IMHO >> a driver should be able to bind ordinary UBOs to all buffer slots. This >> is really not a nice burden to put on state trackers to do something >> special for just slot 0. The gallium API should stay reasonable imho, >> that's a bit too much custom tailoring for GL for my liking. >> >> Maybe I'm missing something but I can't quite see why you can't handle >> this transparently inside the driver. Can't you just create a different >> shader depending on what kind of buffer is bound or what's the problem? >> (You wouldn't expect it to change therefore you should not have to >> recompile.) > > We don't recompile shaders in the vast majority of cases. When shader > compilation stalls rendering, the gaming experience is destroyed. > > There is no alternative. Our shader ABI will be set up such that it > only has 32-bit pointers in shader registers. There are > performance-related reasons for that. That seems to be quite limited, why can't you have a shader ABI which can do either 32 or 64 bit pointers? >>> >>> Good questions. GCN shaders have only 16 dwords of constant memory >>> (GFX9 has 32). There are no shader resource slots and the pixel shader >>> is the only one to have real inputs. All other stages don't have any >>> shader inputs except for system values. >>> >>> The 16 dwords contain pointers and states to load inputs and load >>> descriptions of resource slots from memory. One of the pointers >>> sometimes points to constant buffer 0. If it's a VS, there are only 13 >>> dwords, because 3 are reserved for baseinstance, basevertex, and >>> drawID. We can also put some other data into that constant memory to >>> skip load instructions. There is a huge incentive to free those >>> precious dwords and use them for something else, like skipping some >>> load instructions. I've been also considering 16-bit pointers (e.g. >>> 32-bit pointers aligned to 64KB). >>> >> >> Ok, so for other buffers you can't really do anything special? You just >> go through a pointer to array-of-pointer lookup? > > By default, the shader gets a pointer that points to a merged list of > constant buffer and shader buffer descriptions in memory. If a shader > only uses constant buffer 0 and no shader buffers, that pointer points > to constant buffer 0 directly. Ahh so you can easily guarantee a 32bit pointer if you use the descriptor list (as that's a driver-internal allocation), in which case the actual buffer address size doesn't matter (?), but not if you use the optimization when only constant buffer 0 is used? Albeit that also means you can't do any such optimization for other cases (say, a simple shader only using UBO 0 as that will end up as constant buffer 1, without a guaranteed 32bit address). So I guess indeed if that optimization is worth it, your options are limited (if you don't want a shader dependency on the actual type of buffer bound). Roland > >> I thought "proper" apps would just use UBOs for everything these days >> (hence nothing really much need for tuning slot 0). But maybe that's not >> actually true... I can see that you'd want to optimize usage of this >> precious space. I suppose GL doesn't give you much help there with its >> iffy buffer handling. > > Yes, games use UBOs. > > Marek > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
On Sat, Feb 3, 2018 at 2:55 AM, Roland Scheidegger wrote: > Am 03.02.2018 um 00:31 schrieb Marek Olšák: >> On Sat, Feb 3, 2018 at 12:01 AM, Roland Scheidegger >> wrote: >>> Am 02.02.2018 um 23:39 schrieb Marek Olšák: On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger wrote: > Am 02.02.2018 um 21:48 schrieb Marek Olšák: >> Hi, >> >> This is the second and hopefully final version of 32-bit pointer >> support for radeonsi. >> >> Constant buffer 0 now has restrictions on which buffers can be set >> in that slot. >> >> I plan to push this when my LLVM patch lands in 6.0 (hopefully it >> will be accepted there). >> >> There will also be a dependency on new libdrm (not included in this >> series). >> >> Please review. >> > > From a api cleanliness point of view, I don't like this much. > First, you're making the hack case the default and even require it. IMHO > a driver should be able to bind ordinary UBOs to all buffer slots. This > is really not a nice burden to put on state trackers to do something > special for just slot 0. The gallium API should stay reasonable imho, > that's a bit too much custom tailoring for GL for my liking. > > Maybe I'm missing something but I can't quite see why you can't handle > this transparently inside the driver. Can't you just create a different > shader depending on what kind of buffer is bound or what's the problem? > (You wouldn't expect it to change therefore you should not have to > recompile.) We don't recompile shaders in the vast majority of cases. When shader compilation stalls rendering, the gaming experience is destroyed. There is no alternative. Our shader ABI will be set up such that it only has 32-bit pointers in shader registers. There are performance-related reasons for that. >>> >>> That seems to be quite limited, why can't you have a shader ABI which >>> can do either 32 or 64 bit pointers? >> >> Good questions. GCN shaders have only 16 dwords of constant memory >> (GFX9 has 32). There are no shader resource slots and the pixel shader >> is the only one to have real inputs. All other stages don't have any >> shader inputs except for system values. >> >> The 16 dwords contain pointers and states to load inputs and load >> descriptions of resource slots from memory. One of the pointers >> sometimes points to constant buffer 0. If it's a VS, there are only 13 >> dwords, because 3 are reserved for baseinstance, basevertex, and >> drawID. We can also put some other data into that constant memory to >> skip load instructions. There is a huge incentive to free those >> precious dwords and use them for something else, like skipping some >> load instructions. I've been also considering 16-bit pointers (e.g. >> 32-bit pointers aligned to 64KB). >> > > Ok, so for other buffers you can't really do anything special? You just > go through a pointer to array-of-pointer lookup? By default, the shader gets a pointer that points to a merged list of constant buffer and shader buffer descriptions in memory. If a shader only uses constant buffer 0 and no shader buffers, that pointer points to constant buffer 0 directly. > I thought "proper" apps would just use UBOs for everything these days > (hence nothing really much need for tuning slot 0). But maybe that's not > actually true... I can see that you'd want to optimize usage of this > precious space. I suppose GL doesn't give you much help there with its > iffy buffer handling. Yes, games use UBOs. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
Am 03.02.2018 um 00:31 schrieb Marek Olšák: > On Sat, Feb 3, 2018 at 12:01 AM, Roland Scheidegger > wrote: >> Am 02.02.2018 um 23:39 schrieb Marek Olšák: >>> On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger >>> wrote: Am 02.02.2018 um 21:48 schrieb Marek Olšák: > Hi, > > This is the second and hopefully final version of 32-bit pointer > support for radeonsi. > > Constant buffer 0 now has restrictions on which buffers can be set > in that slot. > > I plan to push this when my LLVM patch lands in 6.0 (hopefully it > will be accepted there). > > There will also be a dependency on new libdrm (not included in this > series). > > Please review. > From a api cleanliness point of view, I don't like this much. First, you're making the hack case the default and even require it. IMHO a driver should be able to bind ordinary UBOs to all buffer slots. This is really not a nice burden to put on state trackers to do something special for just slot 0. The gallium API should stay reasonable imho, that's a bit too much custom tailoring for GL for my liking. Maybe I'm missing something but I can't quite see why you can't handle this transparently inside the driver. Can't you just create a different shader depending on what kind of buffer is bound or what's the problem? (You wouldn't expect it to change therefore you should not have to recompile.) >>> >>> We don't recompile shaders in the vast majority of cases. When shader >>> compilation stalls rendering, the gaming experience is destroyed. >>> >>> There is no alternative. Our shader ABI will be set up such that it >>> only has 32-bit pointers in shader registers. There are >>> performance-related reasons for that. >> >> That seems to be quite limited, why can't you have a shader ABI which >> can do either 32 or 64 bit pointers? > > Good questions. GCN shaders have only 16 dwords of constant memory > (GFX9 has 32). There are no shader resource slots and the pixel shader > is the only one to have real inputs. All other stages don't have any > shader inputs except for system values. > > The 16 dwords contain pointers and states to load inputs and load > descriptions of resource slots from memory. One of the pointers > sometimes points to constant buffer 0. If it's a VS, there are only 13 > dwords, because 3 are reserved for baseinstance, basevertex, and > drawID. We can also put some other data into that constant memory to > skip load instructions. There is a huge incentive to free those > precious dwords and use them for something else, like skipping some > load instructions. I've been also considering 16-bit pointers (e.g. > 32-bit pointers aligned to 64KB). > Ok, so for other buffers you can't really do anything special? You just go through a pointer to array-of-pointer lookup? I thought "proper" apps would just use UBOs for everything these days (hence nothing really much need for tuning slot 0). But maybe that's not actually true... I can see that you'd want to optimize usage of this precious space. I suppose GL doesn't give you much help there with its iffy buffer handling. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
On Sat, Feb 3, 2018 at 12:01 AM, Roland Scheidegger wrote: > Am 02.02.2018 um 23:39 schrieb Marek Olšák: >> On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger >> wrote: >>> Am 02.02.2018 um 21:48 schrieb Marek Olšák: Hi, This is the second and hopefully final version of 32-bit pointer support for radeonsi. Constant buffer 0 now has restrictions on which buffers can be set in that slot. I plan to push this when my LLVM patch lands in 6.0 (hopefully it will be accepted there). There will also be a dependency on new libdrm (not included in this series). Please review. >>> >>> From a api cleanliness point of view, I don't like this much. >>> First, you're making the hack case the default and even require it. IMHO >>> a driver should be able to bind ordinary UBOs to all buffer slots. This >>> is really not a nice burden to put on state trackers to do something >>> special for just slot 0. The gallium API should stay reasonable imho, >>> that's a bit too much custom tailoring for GL for my liking. >>> >>> Maybe I'm missing something but I can't quite see why you can't handle >>> this transparently inside the driver. Can't you just create a different >>> shader depending on what kind of buffer is bound or what's the problem? >>> (You wouldn't expect it to change therefore you should not have to >>> recompile.) >> >> We don't recompile shaders in the vast majority of cases. When shader >> compilation stalls rendering, the gaming experience is destroyed. >> >> There is no alternative. Our shader ABI will be set up such that it >> only has 32-bit pointers in shader registers. There are >> performance-related reasons for that. > > That seems to be quite limited, why can't you have a shader ABI which > can do either 32 or 64 bit pointers? Good questions. GCN shaders have only 16 dwords of constant memory (GFX9 has 32). There are no shader resource slots and the pixel shader is the only one to have real inputs. All other stages don't have any shader inputs except for system values. The 16 dwords contain pointers and states to load inputs and load descriptions of resource slots from memory. One of the pointers sometimes points to constant buffer 0. If it's a VS, there are only 13 dwords, because 3 are reserved for baseinstance, basevertex, and drawID. We can also put some other data into that constant memory to skip load instructions. There is a huge incentive to free those precious dwords and use them for something else, like skipping some load instructions. I've been also considering 16-bit pointers (e.g. 32-bit pointers aligned to 64KB). Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
On Fri, Feb 2, 2018 at 11:39 PM, Marek Olšák wrote: > On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger > wrote: >> Am 02.02.2018 um 21:48 schrieb Marek Olšák: >>> Hi, >>> >>> This is the second and hopefully final version of 32-bit pointer >>> support for radeonsi. >>> >>> Constant buffer 0 now has restrictions on which buffers can be set >>> in that slot. >>> >>> I plan to push this when my LLVM patch lands in 6.0 (hopefully it >>> will be accepted there). >>> >>> There will also be a dependency on new libdrm (not included in this >>> series). >>> >>> Please review. >>> >> >> From a api cleanliness point of view, I don't like this much. >> First, you're making the hack case the default and even require it. IMHO >> a driver should be able to bind ordinary UBOs to all buffer slots. This >> is really not a nice burden to put on state trackers to do something >> special for just slot 0. The gallium API should stay reasonable imho, >> that's a bit too much custom tailoring for GL for my liking. >> >> Maybe I'm missing something but I can't quite see why you can't handle >> this transparently inside the driver. Can't you just create a different >> shader depending on what kind of buffer is bound or what's the problem? >> (You wouldn't expect it to change therefore you should not have to >> recompile.) > > We don't recompile shaders in the vast majority of cases. When shader > compilation stalls rendering, the gaming experience is destroyed. > > There is no alternative. Our shader ABI will be set up such that it > only has 32-bit pointers in shader registers. There are > performance-related reasons for that. > > We'll handle maintenance and testing of this feature. You won't have > to do anything. The CAP is only a formality. In reality, we only have to fix vl_compositor.c to use pipe_context::const_uploader::flags (or the CAP) and that's it. All other code we care about is unaffected (GL, VDPAU, VAAPI, OpenMax, Nine). There are no users binding a real buffer as constant buffer 0 other than VL and XA. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
Am 02.02.2018 um 23:39 schrieb Marek Olšák: > On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger > wrote: >> Am 02.02.2018 um 21:48 schrieb Marek Olšák: >>> Hi, >>> >>> This is the second and hopefully final version of 32-bit pointer >>> support for radeonsi. >>> >>> Constant buffer 0 now has restrictions on which buffers can be set >>> in that slot. >>> >>> I plan to push this when my LLVM patch lands in 6.0 (hopefully it >>> will be accepted there). >>> >>> There will also be a dependency on new libdrm (not included in this >>> series). >>> >>> Please review. >>> >> >> From a api cleanliness point of view, I don't like this much. >> First, you're making the hack case the default and even require it. IMHO >> a driver should be able to bind ordinary UBOs to all buffer slots. This >> is really not a nice burden to put on state trackers to do something >> special for just slot 0. The gallium API should stay reasonable imho, >> that's a bit too much custom tailoring for GL for my liking. >> >> Maybe I'm missing something but I can't quite see why you can't handle >> this transparently inside the driver. Can't you just create a different >> shader depending on what kind of buffer is bound or what's the problem? >> (You wouldn't expect it to change therefore you should not have to >> recompile.) > > We don't recompile shaders in the vast majority of cases. When shader > compilation stalls rendering, the gaming experience is destroyed. > > There is no alternative. Our shader ABI will be set up such that it > only has 32-bit pointers in shader registers. There are > performance-related reasons for that. That seems to be quite limited, why can't you have a shader ABI which can do either 32 or 64 bit pointers? > > We'll handle maintenance and testing of this feature. You won't have > to do anything. > Note that on some apis, there's no way state trackers can do what you want. For instance, with a d3d10 state tracker, you simply can't tell that you're going to bind some buffer to constant slot 0 - there is absolutely nothing special about constant slot 0 (just like with gallium until now). (You could, of course, avoid potential problems with such a hypothetical state tracker by just avoiding slot 0 altogether, albeit you probably then don't expose enough ordinary slots). (I would actually expect for d3d10-style constant buffers you'd wanted to use 32bit pointers for all of them in any case, not just those at slot 0.) But well, it won't really affect anything but radeonsi, so while I think this kind of interface is a mistake and ugly as hell, feel free to stick to it... Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger wrote: > Am 02.02.2018 um 21:48 schrieb Marek Olšák: >> Hi, >> >> This is the second and hopefully final version of 32-bit pointer >> support for radeonsi. >> >> Constant buffer 0 now has restrictions on which buffers can be set >> in that slot. >> >> I plan to push this when my LLVM patch lands in 6.0 (hopefully it >> will be accepted there). >> >> There will also be a dependency on new libdrm (not included in this >> series). >> >> Please review. >> > > From a api cleanliness point of view, I don't like this much. > First, you're making the hack case the default and even require it. IMHO > a driver should be able to bind ordinary UBOs to all buffer slots. This > is really not a nice burden to put on state trackers to do something > special for just slot 0. The gallium API should stay reasonable imho, > that's a bit too much custom tailoring for GL for my liking. > > Maybe I'm missing something but I can't quite see why you can't handle > this transparently inside the driver. Can't you just create a different > shader depending on what kind of buffer is bound or what's the problem? > (You wouldn't expect it to change therefore you should not have to > recompile.) We don't recompile shaders in the vast majority of cases. When shader compilation stalls rendering, the gaming experience is destroyed. There is no alternative. Our shader ABI will be set up such that it only has 32-bit pointers in shader registers. There are performance-related reasons for that. We'll handle maintenance and testing of this feature. You won't have to do anything. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
Am 02.02.2018 um 21:48 schrieb Marek Olšák: > Hi, > > This is the second and hopefully final version of 32-bit pointer > support for radeonsi. > > Constant buffer 0 now has restrictions on which buffers can be set > in that slot. > > I plan to push this when my LLVM patch lands in 6.0 (hopefully it > will be accepted there). > > There will also be a dependency on new libdrm (not included in this > series). > > Please review. > From a api cleanliness point of view, I don't like this much. First, you're making the hack case the default and even require it. IMHO a driver should be able to bind ordinary UBOs to all buffer slots. This is really not a nice burden to put on state trackers to do something special for just slot 0. The gallium API should stay reasonable imho, that's a bit too much custom tailoring for GL for my liking. Maybe I'm missing something but I can't quite see why you can't handle this transparently inside the driver. Can't you just create a different shader depending on what kind of buffer is bound or what's the problem? (You wouldn't expect it to change therefore you should not have to recompile.) Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes
This series actually has 10 patches. See 8/7, 9/7, 10/7. Marek On Fri, Feb 2, 2018 at 9:48 PM, Marek Olšák wrote: > Hi, > > This is the second and hopefully final version of 32-bit pointer > support for radeonsi. > > Constant buffer 0 now has restrictions on which buffers can be set > in that slot. > > I plan to push this when my LLVM patch lands in 6.0 (hopefully it > will be accepted there). > > There will also be a dependency on new libdrm (not included in this > series). > > Please review. > > Marek > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
