[Mesa-dev] [PATCH 3/9] glsl: Combine AST-level and IR-level parameter mode checking loops.
Kenneth Graunke
kenneth at whitecape.org
Wed Mar 28 20:33:02 PDT 2012
generate_call() and ast_function_expression::hir() both tried to verify
that 'out' and 'inout' parameters used l-values. Irritatingly, it
turned out that this was not redundant; both checks caught -some- cases.
This patch combines the two into a single "complete" function that does
all the parameter mode checking. It also adds a comment clarifying why
AST-level checking is necessary in the first place.
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
src/glsl/ast_function.cpp | 167 +++++++++++++++++++++++----------------------
1 files changed, 85 insertions(+), 82 deletions(-)
Again, sorry the diff is hard to read. It's not hard to compare if you
apply it, look at the two old loops, and then the new one. It should be
functionally equivalent.
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 2ad8fba..9ffbce6 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -94,76 +94,116 @@ prototype_string(const glsl_type *return_type, const char *name,
}
/**
- * If a function call is generated, \c call_ir will point to it on exit.
- * Otherwise \c call_ir will be set to \c NULL.
+ * Verify that 'out' and 'inout' actual parameters are lvalues. Also, verify
+ * that 'const_in' formal parameters (an extension in our IR) correspond to
+ * ir_constant actual parameters.
*/
-static ir_rvalue *
-generate_call(exec_list *instructions, ir_function_signature *sig,
- YYLTYPE *loc, exec_list *actual_parameters,
- ir_call **call_ir,
- struct _mesa_glsl_parse_state *state)
+static bool
+verify_parameter_modes(_mesa_glsl_parse_state *state,
+ ir_function_signature *sig,
+ exec_list &actual_ir_parameters,
+ exec_list &actual_ast_parameters)
{
- void *ctx = state;
- exec_list post_call_conversions;
+ exec_node *actual_ir_node = actual_ir_parameters.head;
+ exec_node *actual_ast_node = actual_ast_parameters.head;
- *call_ir = NULL;
+ foreach_list(formal_node, &sig->parameters) {
+ /* The lists must be the same length. */
+ assert(!actual_ir_node->is_tail_sentinel());
+ assert(!actual_ast_node->is_tail_sentinel());
- /* Verify that 'out' and 'inout' actual parameters are lvalues. This
- * isn't done in ir_function::matching_signature because that function
- * cannot generate the necessary diagnostics.
- *
- * Also, validate that 'const_in' formal parameters (an extension of our
- * IR) correspond to ir_constant actual parameters.
- *
- * Also, perform implicit conversion of arguments. Note: to implicitly
- * convert out parameters, we need to place them in a temporary
- * variable, and do the conversion after the call takes place. Since we
- * haven't emitted the call yet, we'll place the post-call conversions
- * in a temporary exec_list, and emit them later.
- */
- exec_list_iterator actual_iter = actual_parameters->iterator();
- exec_list_iterator formal_iter = sig->parameters.iterator();
-
- while (actual_iter.has_next()) {
- ir_rvalue *actual = (ir_rvalue *) actual_iter.get();
- ir_variable *formal = (ir_variable *) formal_iter.get();
+ const ir_variable *const formal = (ir_variable *) formal_node;
+ const ir_rvalue *const actual = (ir_rvalue *) actual_ir_node;
+ const ast_expression *const actual_ast =
+ exec_node_data(ast_expression, actual_ast_node, link);
- assert(actual != NULL);
- assert(formal != NULL);
+ /* FIXME: 'loc' is incorrect (as of 2011-01-21). It is always
+ * FIXME: 0:0(0).
+ */
+ YYLTYPE loc = actual_ast->get_location();
- if (formal->mode == ir_var_const_in && !actual->as_constant()) {
- _mesa_glsl_error(loc, state,
- "parameter `%s' must be a constant expression",
+ /* Verify that 'const_in' parameters are ir_constants. */
+ if (formal->mode == ir_var_const_in &&
+ actual->ir_type != ir_type_constant) {
+ _mesa_glsl_error(&loc, state,
+ "parameter `in %s' must be a constant expression",
formal->name);
- return ir_call::get_error_instruction(ctx);
+ return false;
}
- if ((formal->mode == ir_var_out)
- || (formal->mode == ir_var_inout)) {
+ /* Verify that 'out' and 'inout' actual parameters are lvalues. */
+ if (formal->mode == ir_var_out || formal->mode == ir_var_inout) {
const char *mode = NULL;
switch (formal->mode) {
case ir_var_out: mode = "out"; break;
case ir_var_inout: mode = "inout"; break;
default: assert(false); break;
}
- /* FIXME: 'loc' is incorrect (as of 2011-01-21). It is always
- * FIXME: 0:0(0).
+
+ /* This AST-based check catches errors like f(i++). The IR-based
+ * is_lvalue() is insufficient because the actual parameter at the
+ * IR-level is just a temporary value, which is an l-value.
*/
+ if (actual_ast->non_lvalue_description != NULL) {
+ _mesa_glsl_error(&loc, state,
+ "function parameter '%s %s' references a %s",
+ mode, formal->name,
+ actual_ast->non_lvalue_description);
+ return false;
+ }
+
if (actual->variable_referenced()
&& actual->variable_referenced()->read_only) {
- _mesa_glsl_error(loc, state,
+ _mesa_glsl_error(&loc, state,
"function parameter '%s %s' references the "
"read-only variable '%s'",
mode, formal->name,
actual->variable_referenced()->name);
-
+ return false;
} else if (!actual->is_lvalue()) {
- _mesa_glsl_error(loc, state,
+ _mesa_glsl_error(&loc, state,
"function parameter '%s %s' is not an lvalue",
mode, formal->name);
+ return false;
}
}
+ actual_ir_node = actual_ir_node->next;
+ actual_ast_node = actual_ast_node->next;
+ }
+ return true;
+}
+
+/**
+ * If a function call is generated, \c call_ir will point to it on exit.
+ * Otherwise \c call_ir will be set to \c NULL.
+ */
+static ir_rvalue *
+generate_call(exec_list *instructions, ir_function_signature *sig,
+ YYLTYPE *loc, exec_list *actual_parameters,
+ ir_call **call_ir,
+ struct _mesa_glsl_parse_state *state)
+{
+ void *ctx = state;
+ exec_list post_call_conversions;
+
+ *call_ir = NULL;
+
+ /* Perform implicit conversion of arguments. For out parameters, we need
+ * to place them in a temporary variable and do the conversion after the
+ * call takes place. Since we haven't emitted the call yet, we'll place
+ * the post-call conversions in a temporary exec_list, and emit them later.
+ */
+ exec_list_iterator actual_iter = actual_parameters->iterator();
+ exec_list_iterator formal_iter = sig->parameters.iterator();
+
+ while (actual_iter.has_next()) {
+ ir_rvalue *actual = (ir_rvalue *) actual_iter.get();
+ ir_variable *formal = (ir_variable *) formal_iter.get();
+
+ assert(actual != NULL);
+ assert(formal != NULL);
+
if (formal->type->is_numeric() || formal->type->is_boolean()) {
switch (formal->mode) {
case ir_var_const_in:
@@ -1466,51 +1506,14 @@ ast_function_expression::hir(exec_list *instructions,
if (sig == NULL) {
no_matching_function_error(func_name, &loc, &actual_parameters, state);
value = ir_call::get_error_instruction(ctx);
+ } else if (!verify_parameter_modes(state, sig, actual_parameters, this->expressions)) {
+ /* an error has already been emitted */
+ value = ir_call::get_error_instruction(ctx);
} else {
value = generate_call(instructions, sig, &loc, &actual_parameters,
&call, state);
}
- if (call != NULL) {
- /* If a function was found, make sure that none of the 'out' or 'inout'
- * parameters violate the extra l-value rules.
- */
- ir_function_signature *f = call->get_callee();
- assert(f != NULL);
-
- exec_node *formal_node = f->parameters.head;
-
- foreach_list (actual_node, &this->expressions) {
- /* Both parameter lists had better be the same length!
- */
- assert(!actual_node->is_tail_sentinel());
-
- const ir_variable *const formal_parameter =
- (ir_variable *) formal_node;
- const ast_expression *const actual_parameter =
- exec_node_data(ast_expression, actual_node, link);
-
- if ((formal_parameter->mode == ir_var_out
- || formal_parameter->mode == ir_var_inout)
- && actual_parameter->non_lvalue_description != NULL) {
- YYLTYPE loc = actual_parameter->get_location();
-
- _mesa_glsl_error(&loc, state,
- "function parameter '%s %s' references a %s",
- (formal_parameter->mode == ir_var_out)
- ? "out" : "inout",
- formal_parameter->name,
- actual_parameter->non_lvalue_description);
- return ir_call::get_error_instruction(ctx);
- }
-
- /* Only advance the formal_node pointer here because the
- * foreach_list business already advances actual_node.
- */
- formal_node = formal_node->next;
- }
- }
-
return value;
}
--
1.7.7.6
More information about the mesa-dev
mailing list