[Mesa-dev] [PATCH 3/6] glsl: move layout qualifier validation out of the parser
Timothy Arceri
t_arceri at yahoo.com.au
Thu Nov 5 03:17:33 PST 2015
From: Timothy Arceri <timothy.arceri at collabora.com>
This is in preperation for compile-time constant support.
Also fix up the locations for some of the extension checking
error messages in the parser. We now correctly give the location
of the layout qualifier identifier rather than the integer constant.
The validation is moved to two locations, for validation on variables the
checks are moved to the ast to hir pass and for qualifiers that apply to the
shader the validation is moved into glsl_parser_extras.cpp.
In order to do validation at the later stage in glsl_parser_extras.cpp we
need to temporarily add a field in ast_type_qualifier to keep track of the
parser location, this will be removed in a following patch when we
introduce a new type for storing the comiple-time qualifiers.
Also as the set_shader_inout_layout() function in glsl parser extras is
normally called after all validation is done we need to move the code that
sets CompileStatus and InfoLog otherwise the newly moved error messages will
be ignored.
---
src/glsl/ast.h | 2 +
src/glsl/ast_to_hir.cpp | 101 ++++++++++++++++++++++++++--------------
src/glsl/ast_type.cpp | 2 +
src/glsl/glsl_parser.yy | 100 +++++++++------------------------------
src/glsl/glsl_parser_extras.cpp | 50 +++++++++++++++++---
5 files changed, 137 insertions(+), 118 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index e803e6d..afd2d41 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -553,6 +553,8 @@ struct ast_type_qualifier {
uint64_t i;
} flags;
+ struct YYLTYPE *loc;
+
/** Precision of the type (highp/medium/lowp). */
unsigned precision:2;
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 0306530..8fb8875 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2414,6 +2414,12 @@ validate_explicit_location(const struct ast_type_qualifier *qual,
{
bool fail = false;
+ if (qual->location < 0) {
+ _mesa_glsl_error(loc, state, "invalid location %d specified",
+ qual->location);
+ return;
+ }
+
/* Checks for GL_ARB_explicit_uniform_location. */
if (qual->flags.q.uniform) {
if (!state->check_explicit_uniform_location_allowed(loc, var))
@@ -2537,6 +2543,18 @@ validate_explicit_location(const struct ast_type_qualifier *qual,
assert(!"Unexpected shader type");
break;
}
+ }
+}
+
+static void
+validate_layout_qualifiers(const struct ast_type_qualifier *qual,
+ ir_variable *var,
+ struct _mesa_glsl_parse_state *state,
+ YYLTYPE *loc)
+{
+ if (qual->flags.q.explicit_location) {
+
+ validate_explicit_location(qual, var, state, loc);
if (qual->flags.q.explicit_index) {
/* From the GLSL 4.30 specification, section 4.4.2 (Output
@@ -2556,6 +2574,38 @@ validate_explicit_location(const struct ast_type_qualifier *qual,
var->data.index = qual->index;
}
}
+ } else if (qual->flags.q.explicit_index) {
+ _mesa_glsl_error(loc, state, "explicit index requires explicit location");
+ }
+
+ if (qual->flags.q.explicit_binding &&
+ validate_binding_qualifier(state, loc, var->type, qual)) {
+ var->data.explicit_binding = true;
+ var->data.binding = qual->binding;
+ }
+
+ if (var->type->contains_atomic()) {
+ if (var->data.mode == ir_var_uniform) {
+ if (var->data.explicit_binding) {
+ unsigned *offset =
+ &state->atomic_counter_offsets[var->data.binding];
+
+ if (*offset % ATOMIC_COUNTER_SIZE)
+ _mesa_glsl_error(loc, state,
+ "misaligned atomic counter offset");
+
+ var->data.atomic.offset = *offset;
+ *offset += var->type->atomic_size();
+
+ } else {
+ _mesa_glsl_error(loc, state,
+ "atomic counters require explicit binding point");
+ }
+ } 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");
+ }
}
}
@@ -2935,41 +2985,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
state->fs_redeclares_gl_fragcoord_with_no_layout_qualifiers;
}
- if (qual->flags.q.explicit_location) {
- validate_explicit_location(qual, var, state, loc);
- } else if (qual->flags.q.explicit_index) {
- _mesa_glsl_error(loc, state, "explicit index requires explicit location");
- }
-
- if (qual->flags.q.explicit_binding &&
- validate_binding_qualifier(state, loc, var->type, qual)) {
- var->data.explicit_binding = true;
- var->data.binding = qual->binding;
- }
-
- if (var->type->contains_atomic()) {
- if (var->data.mode == ir_var_uniform) {
- if (var->data.explicit_binding) {
- unsigned *offset =
- &state->atomic_counter_offsets[var->data.binding];
-
- if (*offset % ATOMIC_COUNTER_SIZE)
- _mesa_glsl_error(loc, state,
- "misaligned atomic counter offset");
-
- var->data.atomic.offset = *offset;
- *offset += var->type->atomic_size();
-
- } else {
- _mesa_glsl_error(loc, state,
- "atomic counters require explicit binding point");
- }
- } 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");
- }
- }
+ validate_layout_qualifiers(qual, var, state, loc);
/* Does the declaration use the deprecated 'attribute' or 'varying'
* keywords?
@@ -5964,6 +5980,14 @@ ast_process_structure_or_interface_block(exec_list *instructions,
fields[i].sample = qual->flags.q.sample ? 1 : 0;
fields[i].patch = qual->flags.q.patch ? 1 : 0;
+ if (qual->flags.q.explicit_stream) {
+ if (qual->stream < 0) {
+ YYLTYPE loc = decl_list->get_location();
+ _mesa_glsl_error(&loc, state, "invalid stream %d specified",
+ qual->stream);
+ }
+ }
+
/* Only save explicitly defined streams in block's field */
fields[i].stream = qual->flags.q.explicit_stream ? qual->stream : -1;
@@ -6922,6 +6946,13 @@ ast_cs_input_layout::hir(exec_list *instructions,
*/
GLuint64 total_invocations = 1;
for (int i = 0; i < 3; i++) {
+ if (this->local_size[i] <= 0) {
+ _mesa_glsl_error(state->in_qualifier->loc, state,
+ "invalid local_size_%c of %d specified",
+ 'x' + i, this->local_size[i]);
+ break;
+ }
+
if (this->local_size[i] > state->ctx->Const.MaxComputeWorkGroupSize[i]) {
_mesa_glsl_error(&loc, state,
"local_size_%c exceeds MAX_COMPUTE_WORK_GROUP_SIZE"
diff --git a/src/glsl/ast_type.cpp b/src/glsl/ast_type.cpp
index 08a4504..53d1023 100644
--- a/src/glsl/ast_type.cpp
+++ b/src/glsl/ast_type.cpp
@@ -310,6 +310,7 @@ ast_type_qualifier::merge_out_qualifier(YYLTYPE *loc,
{
void *mem_ctx = state;
const bool r = this->merge_qualifier(loc, state, q);
+ this->loc = loc;
if (state->stage == MESA_SHADER_TESS_CTRL) {
node = new(mem_ctx) ast_tcs_output_layout(*loc, q.vertices);
@@ -329,6 +330,7 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc,
bool create_cs_ast = false;
ast_type_qualifier valid_in_mask;
valid_in_mask.flags.i = 0;
+ this->loc = loc;
switch (state->stage) {
case MESA_SHADER_TESS_EVAL:
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 4636435..44853b0 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -1446,6 +1446,7 @@ layout_qualifier_id:
if (match_layout_qualifier("location", $1, state) == 0) {
$$.flags.q.explicit_location = 1;
+ $$.location = $3;
if ($$.flags.q.attribute == 1 &&
state->ARB_explicit_attrib_location_warn) {
@@ -1453,24 +1454,11 @@ layout_qualifier_id:
"GL_ARB_explicit_attrib_location layout "
"identifier `%s' used", $1);
}
-
- if ($3 >= 0) {
- $$.location = $3;
- } else {
- _mesa_glsl_error(& @3, state, "invalid location %d specified", $3);
- YYERROR;
- }
}
if (match_layout_qualifier("index", $1, state) == 0) {
$$.flags.q.explicit_index = 1;
-
- if ($3 >= 0) {
- $$.index = $3;
- } else {
- _mesa_glsl_error(& @3, state, "invalid index %d specified", $3);
- YYERROR;
- }
+ $$.index = $3;
}
if ((state->has_420pack() ||
@@ -1489,18 +1477,12 @@ layout_qualifier_id:
if (match_layout_qualifier("max_vertices", $1, state) == 0) {
$$.flags.q.max_vertices = 1;
+ $$.max_vertices = $3;
- if ($3 < 0) {
- _mesa_glsl_error(& @3, state,
- "invalid max_vertices %d specified", $3);
- YYERROR;
- } else {
- $$.max_vertices = $3;
- if (!state->is_version(150, 0)) {
- _mesa_glsl_error(& @3, state,
- "#version 150 max_vertices qualifier "
- "specified", $3);
- }
+ if (!state->is_version(150, 0)) {
+ _mesa_glsl_error(& @1, state,
+ "#version 150 max_vertices qualifier "
+ "specified", $3);
}
}
@@ -1508,15 +1490,8 @@ layout_qualifier_id:
if (match_layout_qualifier("stream", $1, state) == 0 &&
state->check_explicit_attrib_stream_allowed(& @3)) {
$$.flags.q.stream = 1;
-
- if ($3 < 0) {
- _mesa_glsl_error(& @3, state,
- "invalid stream %d specified", $3);
- YYERROR;
- } else {
- $$.flags.q.explicit_stream = 1;
- $$.stream = $3;
- }
+ $$.flags.q.explicit_stream = 1;
+ $$.stream = $3;
}
}
@@ -1528,13 +1503,8 @@ layout_qualifier_id:
for (int i = 0; i < 3; i++) {
if (match_layout_qualifier(local_size_qualifiers[i], $1,
state) == 0) {
- if ($3 <= 0) {
- _mesa_glsl_error(& @3, state,
- "invalid %s of %d specified",
- local_size_qualifiers[i], $3);
- YYERROR;
- } else if (!state->has_compute_shader()) {
- _mesa_glsl_error(& @3, state,
+ if (!state->has_compute_shader()) {
+ _mesa_glsl_error(& @1, state,
"%s qualifier requires GLSL 4.30 or "
"GLSL ES 3.10 or ARB_compute_shader",
local_size_qualifiers[i]);
@@ -1549,48 +1519,24 @@ layout_qualifier_id:
if (match_layout_qualifier("invocations", $1, state) == 0) {
$$.flags.q.invocations = 1;
-
- if ($3 <= 0) {
- _mesa_glsl_error(& @3, state,
- "invalid invocations %d specified", $3);
- YYERROR;
- } else if ($3 > MAX_GEOMETRY_SHADER_INVOCATIONS) {
- _mesa_glsl_error(& @3, state,
- "invocations (%d) exceeds "
- "GL_MAX_GEOMETRY_SHADER_INVOCATIONS", $3);
- YYERROR;
- } else {
- $$.invocations = $3;
- if (!state->is_version(400, 0) &&
- !state->ARB_gpu_shader5_enable) {
- _mesa_glsl_error(& @3, state,
- "GL_ARB_gpu_shader5 invocations "
- "qualifier specified", $3);
- }
+ $$.invocations = $3;
+ if (!state->is_version(400, 0) &&
+ !state->ARB_gpu_shader5_enable) {
+ _mesa_glsl_error(& @1, state,
+ "GL_ARB_gpu_shader5 invocations "
+ "qualifier specified", $3);
}
}
/* Layout qualifiers for tessellation control shaders. */
if (match_layout_qualifier("vertices", $1, state) == 0) {
$$.flags.q.vertices = 1;
-
- if ($3 <= 0) {
- _mesa_glsl_error(& @3, state,
- "invalid vertices (%d) specified", $3);
- YYERROR;
- } else if ($3 > (int)state->Const.MaxPatchVertices) {
- _mesa_glsl_error(& @3, state,
- "vertices (%d) exceeds "
- "GL_MAX_PATCH_VERTICES", $3);
- YYERROR;
- } else {
- $$.vertices = $3;
- if (!state->ARB_tessellation_shader_enable &&
- !state->is_version(400, 0)) {
- _mesa_glsl_error(& @1, state,
- "vertices qualifier requires GLSL 4.00 or "
- "ARB_tessellation_shader");
- }
+ $$.vertices = $3;
+ if (!state->ARB_tessellation_shader_enable &&
+ !state->is_version(400, 0)) {
+ _mesa_glsl_error(& @1, state,
+ "vertices qualifier requires GLSL 4.00 or "
+ "ARB_tessellation_shader");
}
}
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index be344d6..1ecfcdb 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -947,6 +947,14 @@ _mesa_ast_process_interface_block(YYLTYPE *locp,
if (state->stage == MESA_SHADER_GEOMETRY &&
state->has_explicit_attrib_stream()) {
+
+ if (state->out_qualifier->flags.q.explicit_stream) {
+ if (state->out_qualifier->stream < 0) {
+ _mesa_glsl_error(locp, state, "invalid stream %d specified",
+ state->out_qualifier->stream);
+ }
+ }
+
/* Assign global layout's stream value. */
block->layout.flags.q.stream = 1;
block->layout.flags.q.explicit_stream = 0;
@@ -1615,7 +1623,7 @@ void ast_subroutine_list::print(void) const
static void
set_shader_inout_layout(struct gl_shader *shader,
- struct _mesa_glsl_parse_state *state)
+ struct _mesa_glsl_parse_state *state)
{
/* Should have been prevented by the parser. */
if (shader->Stage == MESA_SHADER_TESS_CTRL) {
@@ -1644,8 +1652,19 @@ set_shader_inout_layout(struct gl_shader *shader,
switch (shader->Stage) {
case MESA_SHADER_TESS_CTRL:
shader->TessCtrl.VerticesOut = 0;
- if (state->tcs_output_vertices_specified)
+ if (state->tcs_output_vertices_specified) {
+ if (state->out_qualifier->vertices <= 0) {
+ _mesa_glsl_error(state->out_qualifier->loc, state,
+ "invalid vertices (%d) specified",
+ state->out_qualifier->vertices);
+ } else if (state->out_qualifier->vertices >
+ (int)state->Const.MaxPatchVertices) {
+ _mesa_glsl_error(state->out_qualifier->loc, state,
+ "vertices (%d) exceeds GL_MAX_PATCH_VERTICES",
+ state->out_qualifier->vertices);
+ }
shader->TessCtrl.VerticesOut = state->out_qualifier->vertices;
+ }
break;
case MESA_SHADER_TESS_EVAL:
shader->TessEval.PrimitiveMode = PRIM_UNKNOWN;
@@ -1666,8 +1685,14 @@ set_shader_inout_layout(struct gl_shader *shader,
break;
case MESA_SHADER_GEOMETRY:
shader->Geom.VerticesOut = 0;
- if (state->out_qualifier->flags.q.max_vertices)
+ if (state->out_qualifier->flags.q.max_vertices) {
+ if (state->out_qualifier->max_vertices < 0) {
+ _mesa_glsl_error(state->out_qualifier->loc, state,
+ "invalid max_vertices %d specified",
+ state->out_qualifier->max_vertices);
+ }
shader->Geom.VerticesOut = state->out_qualifier->max_vertices;
+ }
if (state->gs_input_prim_type_specified) {
shader->Geom.InputType = state->in_qualifier->prim_type;
@@ -1682,8 +1707,20 @@ set_shader_inout_layout(struct gl_shader *shader,
}
shader->Geom.Invocations = 0;
- if (state->in_qualifier->flags.q.invocations)
+ if (state->in_qualifier->flags.q.invocations) {
+ if (state->in_qualifier->invocations <= 0) {
+ _mesa_glsl_error(state->in_qualifier->loc, state,
+ "invalid invocations %d specified",
+ state->in_qualifier->invocations);
+ } else if (state->in_qualifier->invocations >
+ MAX_GEOMETRY_SHADER_INVOCATIONS) {
+ _mesa_glsl_error(state->in_qualifier->loc, state,
+ "invocations (%d) exceeds "
+ "GL_MAX_GEOMETRY_SHADER_INVOCATIONS",
+ state->in_qualifier->invocations);
+ }
shader->Geom.Invocations = state->in_qualifier->invocations;
+ }
break;
case MESA_SHADER_COMPUTE:
@@ -1796,8 +1833,6 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
ralloc_free(shader->InfoLog);
shader->symbols = new(shader->ir) glsl_symbol_table;
- shader->CompileStatus = !state->error;
- shader->InfoLog = state->info_log;
shader->Version = state->language_version;
shader->IsES = state->es_shader;
shader->uses_builtin_functions = state->uses_builtin_functions;
@@ -1805,6 +1840,9 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
if (!state->error)
set_shader_inout_layout(shader, state);
+ shader->CompileStatus = !state->error;
+ shader->InfoLog = state->info_log;
+
/* Retain any live IR, but trash the rest. */
reparent_ir(shader->ir, shader->ir);
--
2.4.3
More information about the mesa-dev
mailing list