-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/02/2011 05:38 PM, Paul Berry wrote:
> When an out parameter undergoes an implicit type conversion, we need
> to store it in a temporary, and then after the call completes, convert
> the resulting value. In other words, we convert code like the
> following:
>
> void f(out int x);
> float value;
> f(value);
>
> Into IR that's equivalent to this:
>
> void f(out int x);
> float value;
> int x_converted;
> f(x_converted);
> value = float(x_converted);
>
> This transformation needs to happen during ast-to-IR convertion (as
> opposed to, say, a lowering pass), because it is invalid IR for formal
> and actual parameters to have types that don't match.
I want to digest this code a bit more, but I have a couple minor
comments below.
> Fixes piglit tests spec/glsl-1.20/compiler/qualifiers/out-03.vert and
> spec/glsl-1.20/execution/qualifiers/vs-out-{01,02,03}.shader_test, and
> bug 39651.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=39651
> ---
> src/glsl/ast_function.cpp | 64 +++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 8bcf48d..f945a94 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -134,6 +134,8 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> }
> }
>
> + exec_list post_call_conversions;
> +
> if (sig != NULL) {
> /* Verify that 'out' and 'inout' actual parameters are lvalues. This
> * isn't done in ir_function::matching_signature because that function
> @@ -141,6 +143,13 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> *
> * 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.
It looks like this comment (and the others below) are wrapped to much
less than 80 columns.
> */
> exec_list_iterator actual_iter = actual_parameters->iterator();
> exec_list_iterator formal_iter = sig->parameters.iterator();
> @@ -185,8 +194,50 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> }
>
> if (formal->type->is_numeric() || formal->type->is_boolean()) {
> - ir_rvalue *converted = convert_component(actual, formal->type);
> - actual->replace_with(converted);
> + switch (formal->mode) {
> + case ir_var_in:
> + {
The { should be on the same line as the case, and emacs got the
indentation wonkey because it's not.
> + ir_rvalue *converted
> + = convert_component(actual, formal->type);
> + actual->replace_with(converted);
> + }
> + break;
> + case ir_var_out:
> + if (actual->type != formal->type)
> + {
The { should be on the same line as the if.
> + /* Create a temporary variable to hold the out
> + * parameter. Don't pass a hashtable to clone()
> + * because we don't want this temporary variable to
> + * be in scope.
> + */
> + ir_variable *tmp = formal->clone(ctx, NULL);
> + tmp->mode = ir_var_auto;
ir_var_auto is only used for user-defined variables, and
ir_var_temporary is used for compiler-generated variables. We'd also
usually do something like:
ir_variable *tmp =
new(mem_ctx) ir_variable(formal->type,
"out_parameter_conversion",
ir_var_temporary);
Giving it a name like that makes the origin of the variable clear in
shader dumps. This has proven to be an invaluable debugging tool.
> + instructions->push_tail(tmp);
> + ir_dereference_variable *deref_tmp_1
> + = new(ctx) ir_dereference_variable(tmp);
> + ir_dereference_variable *deref_tmp_2
> + = new(ctx) ir_dereference_variable(tmp);
> + ir_rvalue *converted_tmp
> + = convert_component(deref_tmp_1, actual->type);
> + ir_assignment *assignment
> + = new(ctx) ir_assignment(actual, converted_tmp);
> + post_call_conversions.push_tail(assignment);
> + actual->replace_with(deref_tmp_2);
> + }
> + break;
> + case ir_var_inout:
> + /* Inout parameters should never require conversion,
> + * since that would require an implicit conversion to
> + * exist both to and from the formal parameter type,
> + * and there are no bidirectional implicit
> + * conversions.
> + */
> + assert (actual->type == formal->type);
> + break;
> + default:
> + assert (!"Illegal formal parameter mode");
> + break;
> + }
> }
>
> actual_iter.next();
> @@ -196,11 +247,13 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> /* Always insert the call in the instruction stream, and return a deref
> * of its return val if it returns a value, since we don't know if
> * the rvalue is going to be assigned to anything or not.
> + *
> + * Also insert any out parameter conversions after the call.
> */
> ir_call *call = new(ctx) ir_call(sig, actual_parameters);
> + ir_dereference_variable *deref;
> if (!sig->return_type->is_void()) {
> ir_variable *var;
> - ir_dereference_variable *deref;
>
> var = new(ctx) ir_variable(sig->return_type,
> ralloc_asprintf(ctx, "%s_retval",
> @@ -215,11 +268,12 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> var->constant_value = call->constant_expression_value();
>
> deref = new(ctx) ir_dereference_variable(var);
> - return deref;
> } else {
> instructions->push_tail(call);
> - return NULL;
> + deref = NULL;
> }
> + instructions->append_list(&post_call_conversions);
> + return deref;
> } else {
> char *str = prototype_string(NULL, name, actual_parameters);
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk44pBIACgkQX1gOwKyEAw+8cQCdEHeiCWoywUnYtcr+DYuknNNQ
D30An1q/wyBD1Y7nRiCjw7RmYHTpGZjE
=Vh6j
-----END PGP SIGNATURE-----
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
