On 07/27/2011 10:55 PM, Chad Versace wrote:
> Context
> -------
> In ast_function_expression::hir(), parameter_lists_match() checks if the
> function call's actual parameter list matches the signature's parameter
> list, where the match may require implicit conversion of some arguments.
> To check if an implicit conversion exists between individual arguments,
> type_compare() is used.
>
> Problems
> --------
> type_compare() allowed the following illegal implicit conversions:
> bool -> float
> bvecN -> vecN
>
> int -> uint
> ivecN -> uvecN
>
> uint -> int
> uvecN -> ivecN
>
> Change
> ------
> type_compare() is buggy, so replace it with
> glsl_type::can_be_implicitly_converted_to().
> This comprises a rewrite of parameter_lists_match().
>
> Fixes Piglit
> spec/glsl-1.20/compiler/built-in-functions/outerProduct-bvec*.vert
>
> Note: This is a candidate for the 7.10 and 7.11 branches.
> CC: Ian Romanick <ian.d.roman...@intel.com>
> CC: Kenneth Graunke <kenn...@whitecape.org>
> Signed-off-by: Chad Versace <c...@chad-versace.us>
> ---
> src/glsl/glsl_types.cpp | 23 +++----------------
> src/glsl/ir_function.cpp | 52 +++++++++++++++++++++++++++++----------------
> 2 files changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 9cfdcab..3b97aa9 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -531,25 +531,10 @@ glsl_type::can_implicitly_convert_to(const glsl_type
> *desired) const
> return true;
>
> /* There is no conversion among matrix types. */
> - if (this->matrix_columns != 1 || desired->matrix_columns != 1)
> + if (this->matrix_columns > 1 || desired->matrix_columns > 1)
> return false;
>
> - switch (desired->base_type) {
> - case GLSL_TYPE_UINT:
> - case GLSL_TYPE_INT:
> - case GLSL_TYPE_BOOL:
> - case GLSL_TYPE_SAMPLER:
> - case GLSL_TYPE_STRUCT:
> - case GLSL_TYPE_VOID:
> - case GLSL_TYPE_ERROR:
> - return false;
> -
> - case GLSL_TYPE_FLOAT:
> - return this->is_integer()
> - && this->vector_elements == desired->vector_elements;
> -
> - default:
> - assert(0);
> - return false;
> - }
> + return desired->is_float()
> + && this->is_integer()
> + && this->vector_elements == desired->vector_elements;
> }
I think you meant to merge this into the previous patch. Please do so.
> diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
> index 0f2f1a0..56d50a8 100644
> --- a/src/glsl/ir_function.cpp
> +++ b/src/glsl/ir_function.cpp
> @@ -85,12 +85,25 @@ type_compare(const glsl_type *a, const glsl_type *b)
> }
>
>
> +/**
> + * \brief Check if two parameter lists match.
> + *
> + * \param list_a Parameters of the function definition.
> + * \param list_b Actual parameters passed to the function.
> + * \return If an exact match, return 0.
> + * If an inexact match requiring implicit conversion, return 1.
> + * If not a match, return -1.
> + * \see matching_signature()
> + */
> static int
> parameter_lists_match(const exec_list *list_a, const exec_list *list_b)
> {
> const exec_node *node_a = list_a->head;
> const exec_node *node_b = list_b->head;
> - int total_score = 0;
> +
> + /* This is set to true if there is an inexact match requiring an implicit
> + * conversion. */
> + bool inexact_match = false;
>
> for (/* empty */
> ; !node_a->is_tail_sentinel()
> @@ -106,12 +119,11 @@ parameter_lists_match(const exec_list *list_a, const
> exec_list *list_b)
> const ir_variable *const param = (ir_variable *) node_a;
> const ir_instruction *const actual = (ir_instruction *) node_b;
>
> - /* Determine whether or not the types match. If the types are an
> - * exact match, the match score is zero. If the types don't match
> - * but the actual parameter can be coerced to the type of the declared
> - * parameter, the match score is one.
> - */
> - int score;
> + if (param->type == actual->type)
> + continue;
> + /* Try to find an implicit conversion from actual to param. */
> + inexact_match = true;
> switch ((enum ir_variable_mode)(param->mode)) {
> case ir_var_auto:
> case ir_var_uniform:
> @@ -125,29 +137,28 @@ parameter_lists_match(const exec_list *list_a, const
> exec_list *list_b)
>
> case ir_var_const_in:
> case ir_var_in:
> - score = type_compare(param->type, actual->type);
> - break;
> + if (actual->type->can_implicitly_convert_to(param->type))
> + continue;
> + else
> + return -1;
I think this would be easier to read in the more conventional form:
+ if (!actual->type->can_implicitly_convert_to(param->type))
+ return -1;
+ break;
> case ir_var_out:
> - score = type_compare(actual->type, param->type);
> - break;
> + if (param->type->can_implicitly_convert_to(actual->type))
> + continue;
> + else
> + return -1;
Ditto.
> case ir_var_inout:
> /* Since there are no bi-directional automatic conversions (e.g.,
> * there is int -> float but no float -> int), inout parameters must
> * be exact matches.
> */
> - score = (type_compare(actual->type, param->type) == 0) ? 0 : -1;
> - break;
> + return -1;
>
> default:
> assert(false);
> - }
> -
> - if (score < 0)
> return -1;
> -
> - total_score += score;
> + }
> }
>
> /* If all of the parameters from the other parameter list have been
> @@ -157,7 +168,10 @@ parameter_lists_match(const exec_list *list_a, const
> exec_list *list_b)
> if (!node_b->is_tail_sentinel())
> return -1;
>
> - return total_score;
> + if (inexact_match)
> + return 1;
> + else
> + return 0;
> }
Otherwise, looks good to me.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
