On Fri, 2016-02-12 at 18:45 -0500, Ilia Mirkin wrote: > On Fri, Feb 12, 2016 at 5:53 PM, Timothy Arceri <[email protected] > u> wrote: > > On Thu, 2016-02-11 at 20:10 -0500, Ilia Mirkin wrote: > > > This fixes > > > > > > dEQP-GLES31.functional.uniform_location.negative.atomic_fragment > > > dEQP-GLES31.functional.uniform_location.negative.atomic_vertex > > > > > > Both of which have lines like > > > > > > layout(location = 3, binding = 0, offset = 0) uniform atomic_uint > > > uni0; > > > > > > The ARB_explicit_uniform_location spec makes a very tangential > > > mention > > > regarding atomic counters, but location isn't something that > > > makes > > > sense > > > with them. > > > > > > Signed-off-by: Ilia Mirkin <[email protected]> > > > --- > > > > > > Had no clue where to stick this check... this seemed like as good > > > a > > > place as any. > > > > > > src/compiler/glsl/ast_to_hir.cpp | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/src/compiler/glsl/ast_to_hir.cpp > > > b/src/compiler/glsl/ast_to_hir.cpp > > > index dbeb5c0..9fce06b 100644 > > > --- a/src/compiler/glsl/ast_to_hir.cpp > > > +++ b/src/compiler/glsl/ast_to_hir.cpp > > > @@ -4179,6 +4179,11 @@ ast_declarator_list::hir(exec_list > > > *instructions, > > > state->atomic_counter_offsets[qual_binding] = > > > qual_offset; > > > } > > > } > > > > Maybe we should just make this: > > else { > > _mesa_glsl_error(&loc, state, "invalid atomic counter layout > > qualifier"); > > } > > Nope, that doesn't work. In this case > > layout(location = 3, binding = 0, offset = 0) > > it goes into the if () case above, as these are all merged by the > time > it goes into hir. Also, it's legal to just have binding, in which > case > it'd go into the else, which we don't want either. > > This has to be a standalone condition if I do it here.
Right. I corrected myself on IRC. IMO it should be done something like this. /* Valid atomic layout qualifiers */ ast_type_qualifier atomic_layout_mask; atomic_layout_mask.flags.i = 0; atomic_layout_mask.flags.q.explicit_binding = 1; atomic_layout_mask.flags.q.explicit_offset = 1; atomic_layout_mask.flags.q.unifrom = 1 ; ??? if ((qual->flags.i & ~atomic_layout_mask.flags.i) != 0) _mesa_glsl_error(&loc, state, "invalid atomic counter layout qualifier"); As I don't see why we should be treating location as a special case. > > An alternative, as mentioned on IRC, is to stick it into > apply_explicit_location in the if (uniform) check. Or maybe somewhere > else entirely. > > -ilia _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
