Paul Berry <stereotype...@gmail.com> writes: > On 15 September 2013 00:10, Francisco Jerez <curroje...@riseup.net> wrote: >[...] >> + } else if ((op[0]->type->atomic_size() || >> op[1]->type->atomic_size())) { >> + _mesa_glsl_error(&loc, state, "atomic counter comparisons >> forbidden"); >> + error_emitted = true; >> > > Do we have spec text to back this up? I looked around and couldn't find > anything. It seems like doing equality comparisons on atomic counters is > ill-defined, though (do two counters compare equal if they have equal > values, or if they point to the same counter? Both possibilities seem > dodgy). Maybe we should file a spec bug so we can get clarification from > khronos about what's intended. >
It's implied by the same wording you quoted below, "Except for array indexing, structure field selection, and parenthesis, counters are not allowed to be operands in expressions.". > Note that we currently permit other comparisons that are similarly dodgy > (e.g. comparing samplers). It would be nice to get clarification from > khronos about this too. > >[...] >> @@ -1983,7 +1995,7 @@ apply_type_qualifier_to_variable(const struct >> ast_type_qualifier *qual, >> } >> >> if (qual->flags.q.constant || qual->flags.q.attribute >> - || qual->flags.q.uniform >> + || (qual->flags.q.uniform && var->type != >> glsl_type::atomic_uint_type) >> || (qual->flags.q.varying && (state->target == fragment_shader))) >> var->read_only = 1; >> > > I'm not entirely convinced this is right. An atomic counter, like a > sampler, is a handle to an underlying object, not the underlying object > itself. The handle should be considered read-only even if the object it > refers to is mutable. That way we still prohibit > I guess the question is if we want atomic ir_variables represent the handle of an atomic counter or the atomic counter itself. The latter seemed to make more sense to me but you're right that the spec favours the opposite point of view, I will fix that. > uniform atomic_uint foo; > uniform atomic_uint bar; > void main() { > foo = bar; > } > > >>[...] >> @@ -4475,6 +4533,12 @@ ast_process_structure_or_interface_block(exec_list >> *instructions, >> "uniform in non-default uniform block >> contains sampler"); >> } >> >> + 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: > > "Except for array indexing, structure field selection, and parenthesis, > counters are not allowed to be operands in expressions." > > Which would seem to imply that atomic counters *are* allowed as structure > fields. > That's a tricky question, my impression when I first read this extension was that it was allowed, but the spec seemed to be inconsistent in that regard and it leaves a number of questions unanswered. The wording you're quoting doesn't apply directly to atomic counters as they are defined by the GL 4.2 spec, it refers to opaque types in general: "Except for array indexing, structure member selection, and parentheses, opaque variables are not allowed to be operands in expressions; such use results in a compile-time error." Later on the spec requires atomic counter uniform declarations to have a layout() specification tying them to some specific binding point, which seems to be forbidden in structure member declarations: "[Structure] member declarators may contain precision qualifiers, but use of any other qualifier results in a compile-time error." One could imagine that the structure declaration syntax could be relaxed in cases where a structure contains atomic counters to allow binding and offset qualifiers on the whole structure, like: | struct S { | .... | atomic_uint x; | }; | | layout(binding=X, offset=Y) uniform S foo; | The spec doesn't specify this syntax nor how atomic counters contained in the structure would be supposed to get their bindings and offsets assigned. It seems it would be problematic because of the way the binding and offset qualifiers have different meanings for different type declarations -- E.g. what if you declare a structure containing an atomic counter and an image2D? Are they supposed to get the same binding even though they refer to completely different name spaces? Is the binding number supposed to be post-incremented for each declaration like is the case for image and sampler arrays, or stay constant as in atomic counter arrays? To summarize, the spec doesn't seem to provide any reasonable way to declare structures with atomic counters even though it provides the syntax for atomic structure member selection... For uniform blocks OTOH it's quite clear that they are not allowed: "[In interface blocks] types and declarators are the same as for other input, output, uniform, and buffer variable declarations outside blocks, with these exceptions: [...] opaque types are not allowed." Thanks. > >>[...]
pgpVIDl3aUoWf.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev