-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/01/2011 04:07 PM, Paul Berry wrote:
> The ast-to-hir conversion needs to emit function signatures in two
> circumstances: when a function declaration (or definition) is
> encountered, and when a built-in function is encountered.
>
> To avoid emitting a function signature in an illegal place (such as
> inside a function), emit_function() checked whether we were inside a
> function definition, and if so, emitted the signature before the
> function definition.
>
> However, this didn't cover the case of emitting function signatures
> for built-in functions when those built-in functions are called from
> global scope (e.g. a built-in function called from inside the constant
> integer expression that specifies the length of an array).
>
> This patch changes emit_function() so that it emits function
> signatures at toplevel in all cases.
There's something about this patch that I don't grok. The
state->current_function test in emit function exists specifically to
handle calls to functions (built-in or otherwise) at global scope.
Without that, code such as the snippet below would not work, and text in
the commit log seems to indicate that it shouldn't work. However, I
verified that it does.
#version 120
const float x = cos(3.14159 / 2.0);
> ---
> src/glsl/ast.h | 3 +--
> src/glsl/ast_function.cpp | 2 +-
> src/glsl/ast_to_hir.cpp | 31 ++++++++++++++-----------------
> src/glsl/glsl_parser_extras.h | 6 ++++++
> 4 files changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 878f48b..d1de227 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -730,7 +730,6 @@ _mesa_ast_field_selection_to_hir(const ast_expression
> *expr,
> struct _mesa_glsl_parse_state *state);
>
> void
> -emit_function(_mesa_glsl_parse_state *state, exec_list *instructions,
> - ir_function *f);
> +emit_function(_mesa_glsl_parse_state *state, ir_function *f);
>
> #endif /* AST_H */
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 8bcf48d..34a82f8 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -125,7 +125,7 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> if (f == NULL) {
> f = new(ctx) ir_function(name);
> state->symbols->add_global_function(f);
> - emit_function(state, instructions, f);
> + emit_function(state, f);
> }
>
> f->add_signature(sig->clone_prototype(f, NULL));
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index c0524bf..c1f160e 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -66,6 +66,8 @@ _mesa_ast_to_hir(exec_list *instructions, struct
> _mesa_glsl_parse_state *state)
>
> state->current_function = NULL;
>
> + state->toplevel_ir = instructions;
> +
> /* Section 4.2 of the GLSL 1.20 specification states:
> * "The built-in functions are scoped in a scope outside the global scope
> * users declare global variables in. That is, a shader's global scope,
> @@ -85,6 +87,8 @@ _mesa_ast_to_hir(exec_list *instructions, struct
> _mesa_glsl_parse_state *state)
> ast->hir(instructions, state);
>
> detect_recursion_unlinked(state, instructions);
> +
> + state->toplevel_ir = NULL;
> }
>
>
> @@ -2926,23 +2930,16 @@ ast_parameter_declarator::parameters_to_hir(exec_list
> *ast_parameters,
>
>
> void
> -emit_function(_mesa_glsl_parse_state *state, exec_list *instructions,
> - ir_function *f)
> +emit_function(_mesa_glsl_parse_state *state, ir_function *f)
> {
> - /* Emit the new function header */
> - if (state->current_function == NULL) {
> - instructions->push_tail(f);
> - } else {
> - /* IR invariants disallow function declarations or definitions nested
> - * within other function definitions. Insert the new ir_function
> - * block in the instruction sequence before the ir_function block
> - * containing the current ir_function_signature.
> - */
> - ir_function *const curr =
> - const_cast<ir_function *>(state->current_function->function());
> -
> - curr->insert_before(f);
> - }
> + /* IR invariants disallow function declarations or definitions
> + * nested within other function definitions. But there is no
> + * requirement about the relative order of function declarations
> + * and definitions with respect to one another. So simply insert
> + * the new ir_function block at the end of the toplevel instruction
> + * list.
> + */
> + state->toplevel_ir->push_tail(f);
> }
>
>
> @@ -3069,7 +3066,7 @@ ast_function::hir(exec_list *instructions,
> return NULL;
> }
>
> - emit_function(state, instructions, f);
> + emit_function(state, f);
> }
>
> /* Verify the return type of main() */
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 2f4d3cb..fc392da 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -129,6 +129,12 @@ struct _mesa_glsl_parse_state {
> */
> class ir_function_signature *current_function;
>
> + /**
> + * During AST to IR conversion, pointer to the toplevel IR
> + * instruction list being generated.
> + */
> + exec_list *toplevel_ir;
> +
> /** Have we found a return statement in this function? */
> bool found_return;
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk43TfgACgkQX1gOwKyEAw+IrwCfYvuuQJtoS8RpI0lICMoTgYsm
RrcAn0h9erWBQTPL4i8AkXR+J+Byfawi
=oPRj
-----END PGP SIGNATURE-----
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
