[Mesa-dev] SSBO's in UniformBlocks list?

[Francisco Jerez]

Iago Toral <itoral at igalia.com> writes:

> On Tue, 2015-09-29 at 18:41 +0300, Francisco Jerez wrote:
>> Ilia Mirkin <imirkin at alum.mit.edu> writes:
>> 
>> > On Tue, Sep 29, 2015 at 4:33 AM, Iago Toral <itoral at igalia.com> wrote:
>> >> Hi ilia,
>> >>
>> >> On Tue, 2015-09-29 at 03:53 -0400, Ilia Mirkin wrote:
>> >>> Hi Samuel, and any other onlookers,
>> >>>
>> >>> I was wondering why the decision was made to stick SSBO's onto the
>> >>> same list as constbufs. Seems like they're entirely separate entities,
>> >>> no? Perhaps I'm missing something?
>> >>
>> >> The reason for this was that there is a lot of code in the compiler to
>> >> handle uniform blocks and all the rules for them and we needed the same
>> >> treatment for SSBOs, so that seemed like a reasonable way forward to
>> >> reuse a lot of the code in the compiler front end. I think the only
>> >> place where we needed to make explicit distinctions is when we check for
>> >> resource limits, since these are different for UBOs and SSBOs of course.
>> >> Although UBOs and SSBOs are separate entities they have a lot of
>> >> similarities too, so that did not look like a terrible idea, considering
>> >> the benefits.
>> >
>> > My concern is around indexing... now the per-stage indices are in the
>> > combined UBO/SSBO space -- how do I tease out the individual ones?
>> > Easy enough when you can loop over NumUniformBlocks and just count the
>> > right type, but what about in the shader, where I get the buffer index
>> > in a ir_rvalue?
>> >
>> Yeah, this seems rather dubious to me too.  Even if you had re-used the
>> current gl_uniform_block type for SSBOs for the sake of code-sharing I
>> think it would have made more sense to split them into a different index
>> space, because SSBOs are a different index space at the API level and
>> drivers will want them to be a different index space too.
>
> In the case of i965 at least I think having them combined makes things
> easier because from the driver perspective the only difference between
> SSBOs and UBOs is that the underlying buffer storage has different
> flags. This was the simplest implementation and in theory, translating
> from the shared space to a separate space should be very easy too, so
> drivers that need a separate space should be able to get that as well...
> That's in theory, the problem is that Ilia is saying that in his case
> the compiler backend may not have the information required to make this
> translation, and if this is true then we'll have to rethink this...
>
I think this is largely irrelevant for the i965 driver, code can be
shared either way if shader UBOs and SSBOs are represented using the
same data type: Assuming that function f() is shared among SSBOs and
UBOs, it could also have been shared with separate arrays by changing it
into f(gl_uniform_block[]) and passing the right U/SSBO array as
argument.

Translating from a separate space to a shared index space is trivial and
has O(1) complexity (shared_ssbo_index = separate_ssbo_index +
max_ubo_index), although it's unlikely to be necessary on any HW
arrchitecture I know of.  OTOH translating from the shared space to a
separatee space (as the GL API and Gallium drivers require) involves
iterating over all previous U/SSBOs of the shader and has O(n)
complexity (e.g. as you do here [1]).

[1] https://patchwork.freedesktop.org/patch/60654/

>> I believe that this leads to a bug the i965 implementation -- We expose
>> 12 SSBOs per stage and 12 UBOs per stage, but we only have 12 binding
>> table entries reserved for the block of the binding table currently
>> shared among UBOs and SSBOs, so you might overflow the number of
>> available surface entries if the combined number of UBOs and SSBOs is
>> greater than 12 for some stage.
>
> Yeah, we forgot to update that. I'll send a patch to fix this. Thanks
> Curro!
>
> Iago
>
>> > Thanks,
>> >
>> >   -ilia
>> > _______________________________________________
>> > 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/20150930/3d94ded7/attachment.sig>