On 23 December 2011 14:35, Ian Romanick <i...@freedesktop.org> wrote:
> From: Ian Romanick <ian.d.roman...@intel.com> > > Somethings, like pre-increment operations, were not previously caught. > After the 8.0 release, this code needs some major refactoring and > clean-up. It's a mess. :( > > Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755 > --- > src/glsl/ast_function.cpp | 57 > +++++++++++++++++++++++++++++++++++++++++--- > 1 files changed, 53 insertions(+), 4 deletions(-) > > diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp > index 126b610..4471c76 100644 > --- a/src/glsl/ast_function.cpp > +++ b/src/glsl/ast_function.cpp > @@ -96,6 +96,7 @@ prototype_string(const glsl_type *return_type, const > char *name, > 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) > Another minor suggestion: The semantics of call_ir are: - If generate_call is successful, it is set to the ir_call that is generated. - If an error occurred, it is unchanged. This means that in order for the caller to be robust in the case of error conditions, it must remember to set call_ir to NULL before calling this function. You're doing that, so there's no bug, but I'd feel better about the situation if we set *call_ir to NULL at the top of generate_call rather than in the caller. That way call_ir would behave like a true out parameter even in the case of error. As with my other comment, I'm not married to the idea; either way the series is: Reviewed-by: Paul Berry <stereotype...@gmail.com> > { > void *ctx = state; > @@ -256,10 +257,12 @@ generate_call(exec_list *instructions, > ir_function_signature *sig, > deref = new(ctx) ir_dereference_variable(var); > ir_assignment *assign = new(ctx) ir_assignment(deref, call, NULL); > instructions->push_tail(assign); > + *call_ir = call; > > deref = new(ctx) ir_dereference_variable(var); > } else { > instructions->push_tail(call); > + *call_ir = call; > deref = NULL; > } > instructions->append_list(&post_call_conversions); > @@ -269,6 +272,7 @@ generate_call(exec_list *instructions, > ir_function_signature *sig, > static ir_rvalue * > match_function_by_name(exec_list *instructions, const char *name, > YYLTYPE *loc, exec_list *actual_parameters, > + ir_call **call_ir, > struct _mesa_glsl_parse_state *state) > { > void *ctx = state; > @@ -342,7 +346,8 @@ done: > } > > /* Finally, generate a call instruction. */ > - return generate_call(instructions, sig, loc, actual_parameters, > state); > + return generate_call(instructions, sig, loc, actual_parameters, > + call_ir, state); > } else { > char *str = prototype_string(NULL, name, actual_parameters); > > @@ -1442,9 +1447,53 @@ ast_function_expression::hir(exec_list > *instructions, > process_parameters(instructions, &actual_parameters, > &this->expressions, > state); > > - return match_function_by_name(instructions, > - id->primary_expression.identifier, & > loc, > - &actual_parameters, state); > + ir_call *call = NULL; > + ir_rvalue *const value = > + match_function_by_name(instructions, > + id->primary_expression.identifier, > + &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; > } > > return ir_call::get_error_instruction(ctx); > -- > 1.7.6.4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
