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

[Ilia Mirkin]

On Tue, Jan 19, 2016 at 5:49 AM, Marek Olšák <maraeo at gmail.com> wrote:
> 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.
>
> 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

Everything you say is correct, however it's perfectly legal to have N
maximum binding points as well as N maximum combined buffers. Yes,
they're counting, in essence, different things, but those numbers are
not inconsistent. You could have 10000000 binding points, and 10
maximum combined buffers. The one thing that would never make sense
would be having more than 6 * binding points as the max combined
buffers (assuming compute gets counted).

Anyways, like you say, some optimization should be possible here.
Perhaps Francisco can elaborate as to why he picked these numbers.

  -ilia