[Mesa-dev] [PATCH 0/7] RadeonSI 32-bit pointers v2 & Gallium changes

[Roland Scheidegger]

Am 02.02.2018 um 23:39 schrieb Marek Olšák:
> On Fri, Feb 2, 2018 at 10:26 PM, Roland Scheidegger <sroland at vmware.com> 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