[Mesa-dev] [PATCH v5 45/70] glsl: atomic counters can be declared as buffer-qualified variables

[Francisco Jerez]

Iago Toral <itoral at igalia.com> writes:

> On Mon, 2015-09-28 at 13:45 +0200, Iago Toral wrote:
>> On Mon, 2015-09-28 at 13:13 +0300, Francisco Jerez wrote:
>> > Iago Toral Quiroga <itoral at igalia.com> writes:
>> > 
>> > > From: Kristian Høgsberg <krh at bitplanet.net>
>> > >
>> > > ---
>> > >  src/glsl/ast_to_hir.cpp | 6 +++---
>> > >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> > > index 566cc87..a364aae 100644
>> > > --- a/src/glsl/ast_to_hir.cpp
>> > > +++ b/src/glsl/ast_to_hir.cpp
>> > > @@ -2789,7 +2789,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>> > >     }
>> > >  
>> > >     if (var->type->contains_atomic()) {
>> > > -      if (var->data.mode == ir_var_uniform) {
>> > > +      if (var->data.mode == ir_var_uniform || var->data.mode == ir_var_shader_storage) {
>> > >           if (var->data.explicit_binding) {
>> > >              unsigned *offset =
>> > >                 &state->atomic_counter_offsets[var->data.binding];
>> > > @@ -2807,8 +2807,8 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>> > >           }
>> > >        } else if (var->data.mode != ir_var_function_in) {
>> > >           _mesa_glsl_error(loc, state, "atomic counters may only be declared as "
>> > > -                          "function parameters or uniform-qualified "
>> > > -                          "global variables");
>> > > +                          "function parameters, uniform-qualified or "
>> > > +                          "buffer-qualified global variables");
>> > >        }
>> > >     }
>> > >  
>> > 
>> > Spec quote?  Declaring an atomic counter to have buffer storage is
>> > illegal AFAIK.
>> 
>> I think you are right, the specs don't seem to include any explicit
>> restrictions affecting the allowed types for buffer variables, however,
>> the description of opaque types seems to be in conflict with that:
>> 
>> "The opaque types declare variables that are effectively opaque 
>> handles to other objects. These objects are
>> accessed through built-in functions, not through direct reading or
>> writing of the declared variable.
>> (...)
>> Opaque variables cannot be treated as l-values;(...)"
>> 
>> I am thinking that we probably want to revert this patch and instead
>> check that buffer variables do not contain references to any opaque
>> types and produce a compile-time error if that is the case. I'll send
>> patches for this. Maybe the specs should be amended to mention this
>> explicitly too. Ian, what do you think?
>
> Well, actually we already check that opaque types are disallowed in
> interface blocks, so we only need to revert this patch.
>

Yeah, please do, and feel free to add my R-b.

> Iago
-------------- 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/20150928/26cae080/attachment.sig>