On Sat, 2015-11-07 at 23:16 -0800, Jordan Justen wrote: > On 2015-11-06 19:33:38, Timothy Arceri wrote: > > On Fri, 2015-11-06 at 17:56 -0800, Jordan Justen wrote: > > > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> > > > --- > > > > > > Notes: > > > git://people.freedesktop.org/~jljusten/mesa cs-parse-shared-vars-v1 > > > > > > http://patchwork.freedesktop.org/bundle/jljusten/cs-parse-shared-vars-v1 > > > > > > With these environment overrides: > > > > > > export MESA_GL_VERSION_OVERRIDE=4.3 > > > export MESA_GLSL_VERSION_OVERRIDE=430 > > > export MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader > > > > > > This fixes my recently posted piglit test: > > > > > > tests/spec/arb_compute_shader/compiler/shared-variables.comp > > > http://patchwork.freedesktop.org/patch/63944/ > > > > > > src/glsl/ast_to_hir.cpp | 2 +- > > > src/glsl/glsl_lexer.ll | 2 ++ > > > src/glsl/glsl_parser.yy | 6 ++++++ > > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp > > > index 0306530..dd5ba4e 100644 > > > --- a/src/glsl/ast_to_hir.cpp > > > +++ b/src/glsl/ast_to_hir.cpp > > > @@ -3081,7 +3081,7 @@ apply_type_qualifier_to_variable(const struct > > > ast_type_qualifier *qual, > > > if (qual->flags.q.std140 || > > > qual->flags.q.std430 || > > > qual->flags.q.packed || > > > - qual->flags.q.shared) { > > > + (qual->flags.q.shared && (state->stage != MESA_SHADER_COMPUTE))) > > > { > > > _mesa_glsl_error(loc, state, > > > "uniform and shader storage block layout > > > qualifiers > > > " > > > "std140, std430, packed, and shared can only be > > > " > > > diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll > > > index 2142817..e59f93e 100644 > > > --- a/src/glsl/glsl_lexer.ll > > > +++ b/src/glsl/glsl_lexer.ll > > > @@ -414,6 +414,8 @@ writeonly KEYWORD_WITH_ALT(420, 300, 420, 310, > > > yyextra->ARB_shader_image_lo > > > > > > atomic_uint KEYWORD_WITH_ALT(420, 300, 420, 310, yyextra > > > ->ARB_shader_atomic_counters_enable, ATOMIC_UINT); > > > > > > +shared KEYWORD_WITH_ALT(430, 310, 430, 310, yyextra > > > ->ARB_compute_shader_enable, SHARED); > > > + > > > struct return STRUCT; > > > void return VOID_TOK; > > > > > > diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy > > > index 4636435..2598356 100644 > > > --- a/src/glsl/glsl_parser.yy > > > +++ b/src/glsl/glsl_parser.yy > > > @@ -165,6 +165,7 @@ static bool match_layout_qualifier(const char *s1, > > > const > > > char *s2, > > > %token IMAGE1DSHADOW IMAGE2DSHADOW IMAGE1DARRAYSHADOW > > > IMAGE2DARRAYSHADOW > > > %token COHERENT VOLATILE RESTRICT READONLY WRITEONLY > > > %token ATOMIC_UINT > > > +%token SHARED > > > %token STRUCT VOID_TOK WHILE > > > %token <identifier> IDENTIFIER TYPE_IDENTIFIER NEW_IDENTIFIER > > > %type <identifier> any_identifier > > > @@ -1958,6 +1959,11 @@ memory_qualifier: > > > memset(& $$, 0, sizeof($$)); > > > $$.flags.q.write_only = 1; > > > } > > > + | SHARED > > > + { > > > + memset(& $$, 0, sizeof($$)); > > > + $$.flags.q.shared = 1; > > > + } > > > > Hi Jordan, > > > > This should be in storage_qualifier: rather than memory_qualifier: > > > > Also it should be restricted to the computer shader stage e.g. > > > > if (state->stage == MESA_SHADER_COMPUTE) { > > memset(& $$, 0, sizeof($$)); > > $$.flags.q.shared = 1; > > } else { > > _mesa_glsl_error(&@1, state, "the shared storage qualifiers can > > only > > "be used with compute shaders"); > > } > > > > Maybe add a piglit test to make sure it fails in another stage? > > I tested nvidia, and they don't fail to compile when a variable is > declared as shared in the render stages. > > The spec says: > > "Variables declared as shared may only be used in compute shaders" > > Unfortunately, that doesn't specifically say that it should fail at > the compile step.
Yeah your right, in that case feel free not to add this check. > > I think it is fine for us to fail at the compile phase, but I'm not > sure about tests for this. > > -Jordan _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
