[Mesa-dev] [PATCH 01/10] tgsi: add MEMBAR opcode to handle memoryBarrier* GLSL intrinsics

[Francisco Jerez]

Marek Olšák <maraeo at gmail.com> writes:

> On Tue, Jan 19, 2016 at 3:25 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Mon, Jan 18, 2016 at 6:06 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>> For 1-4,
>>>
>>> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>>>
>>> I'm not very familiar with the code in 2, but the changes seem reasonable.
>>>
>>> Also, and I know this is not your mistake, but still, mtypes.h has:
>>>
>>> struct gl_atomic_buffer_binding
>>>       AtomicBufferBindings[MAX_COMBINED_ATOMIC_BUFFERS];
>>>
>>> But it should be:
>>>
>>> struct gl_atomic_buffer_binding
>>>       AtomicBufferBindings[16 /* or use a proper definition here */];
>>>
>>> It's not possible to use more than MaxAtomicBufferBindings, because
>>> the slots are shared among all shader stages.
>>
>> Besides the suboptimal name, I don't see what's so terribly wrong
>> about this. They're saying there are 15*6 binding points (the value of
>> MAX_COMBINED_ATOMIC_BUFFERS), and that's also the
>> MaxCombinedAtomicBuffers thing. I guess MaxCombinedAtomicBuffers
>> should be N * the max # of bindings? So this is a bit more restrictive
>> than it could be, but... meh.
>
> I don't understand. AtomicBufferBindings are binding points. There is
> a fixed number of binding points shared by all shader stages, for
> example 15. It's not possible to bind a buffer above that. From the
> spec:
>
> "BindBufferBase and BindBufferRange will generate an INVALID_VALUE
> error if <index> is greater than or equal to the value of
> MAX_ATOMIC_COUNTER_BUFFER_BINDINGS".
>
> This is why AtomicBufferBindings[MAX_COMBINED_ATOMIC_BUFFERS] wastes a
> lot of memory.
>
It only wastes any memory if your driver exposes a
GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS less than the
MAX_COMBINED_ATOMIC_BUFFERS macro, which might be necessary due to
hardware restrictions, and even in that case the "waste" will be a
constant amount of memory per context so I'm not particularly concerned
about it, you could avoid it by allocating the array manually based on
the MaxAtomicBufferBindings constant provided by the driver, but it
would be kind of painful.

I didn't choose the binding point array size to be
MAX_COMBINED_ATOMIC_BUFFERS by accident, it's the maximum amount of
atomic counter binding points that can potentially be useful, because
anything greater than that is functionally equivalent to a context with
MAX_COMBINED_ATOMIC_BUFFERS binding points since you won't be able to
set up a pipeline using more than that many atomic buffers.  The i965
driver exposes MAX_COMBINED_ATOMIC_BUFFERS binding points for that
reason.

> MAX_COMBINED_ATOMIC_BUFFERS is about shader references, not bindings.
> You can have 15 global bindings (slots), but 5 shaders can reference
> them, which leads to 15*5 references. This is a thing that only the
> linker should care about. It's only possible to get the maximum if all
> 5 stages use the same slots (because there are only 15). The spec is
> pretty clear about it:
>
> "If an atomic counter buffer is used by multiple shaders, each such
> use counts separately against this combined limit. The combined atomic
> counter buffer use limit can be obtained by calling GetIntegerv with a
> <pname> of MAX_COMBINED_ATOMIC_COUNTER_BUFFERS"
>
> Marek
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160119/84d07ab0/attachment.sig>