[Mesa-dev] [PATCH 10/24] glsl: Implement parser support for atomic counters.

[Francisco Jerez]

Ian Romanick <idr at freedesktop.org> writes:

> On 09/17/2013 03:51 PM, Paul Berry wrote:
>> On 15 September 2013 00:10, Francisco Jerez <currojerez at riseup.net
>> <mailto:currojerez at riseup.net>> wrote:
>>[...]
>> 
>> GLSL 4.20 clarifies that atomic counter offsets must be unique and
>> non-overlapping (see GLSL 4.20 4.4.4.1 "Atomic Counter Layout
>> Qualifiers").  So I tihnk we need a stronger check than this. 
>> Specifically, declarations like the following should be disallowed:
>> 
>> layout(binding=0, offset=16) atomic_uint foo[4]; // Uses offsets 16, 20,
>> 24, 28
>> layout(binding=0, offset=20) atomic_uint bar; // Error: overlaps foo.
>
> I also think this check belongs in the linker.  I don't see a lot of
> value in doing it twice, and we have to verify atomics declared in other
> compilation units.
>
I can move that check back to the linker, you're right that it would
probably be more convenient for us to do so.  The spec isn't very clear
on whether this should be a compile or a link-time error, actually I
implemented this in the linker first but in doubt it seemed desirable to
be able to catch this sort of errors as early as possible, and the spec
mentions explicitly that using atomic buffer bindings over the
implementation's maximum should cause a compile-time error.

>>
>>     +         if (field_type->atomic_size()) {
>>     +            YYLTYPE loc = decl_list->get_location();
>>     +            _mesa_glsl_error(&loc, state, "atomic counter in
>>     structure or "
>>     +                             "uniform block");
>>     +         }
>>     +
>> 
>> Are you sure this is not allowed?  I can't find any spec text to back
>> this up.  All I found was this text from ARB_shader_atomic_counters:
>
> Yeah, this is definitely wrong.  See the text I quoted in reply to patch
> 8.[...]
>
Please read my answer to Paul's post.  It's not clear how atomic
counters declared within a structure would get their binding points and
offsets assigned: binding and offset qualifiers are disallowed within
structs, and using a single layout() qualifier for the whole uniform
struct declaration isn't satisfactory either.  I think we should
disallow them for now, the spec is just too inconsistent...

Thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130918/991e51e9/attachment.pgp>