[Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
Ian Romanick
idr at freedesktop.org
Mon Apr 18 16:36:54 PDT 2011
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/17/2011 11:39 PM, Bryan Cain wrote:
> From the GLSL 1.30 specification, section 4.1.10 ("Implicit Conversions"):
> "There are no implicit conversions between signed and unsigned integers."
>
> However, convert_component() was assuming that conversions between int and uint were implicit.
The conversions applied in convert_component only apply to constructor
parameters. As Ken mentions, in those cases we want
Internally we treat signed and unsigned identically because the
representation is the same. There's no way for an application to
observe the difference between:
ivec4 a = ivec4(5 , 6 , 7 , 8 );
ivec4 b = ivec4(5 , 6 , 7 , 8u);
ivec4 c = ivec4(5 , 6 , 7u, 8 );
ivec4 d = ivec4(5 , 6 , 7u, 8u);
ivec4 e = ivec4(5 , 6u, 7 , 8 );
ivec4 f = ivec4(5 , 6u, 7 , 8u);
ivec4 g = ivec4(5 , 6u, 7u, 8 );
ivec4 h = ivec4(5 , 6u, 7u, 8u);
// etc.
The type checker ensures that operands to operations that could exhibit
differing behavior (e.g., ir_bin_mul) have identical types. That code
lives in arithmetic_result_type in ast_to_hir.cpp. There is similar
code in match_function_by_name in ast_function.cpp.
Do you have any specific test cases in mind that don't produce correct
results with the existing code?
> ---
> src/glsl/ast_function.cpp | 16 ++++++++++++----
> src/glsl/ir.cpp | 8 ++++++++
> src/glsl/ir.h | 2 ++
> src/glsl/ir_constant_expression.cpp | 8 ++++++++
> src/glsl/ir_validate.cpp | 8 ++++++++
> 5 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index e5cb873..cc3f032 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -267,17 +267,25 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type)
> assert(a <= GLSL_TYPE_BOOL);
> assert(b <= GLSL_TYPE_BOOL);
>
> - if ((a == b) || (src->type->is_integer() && desired_type->is_integer()))
> + if (a == b)
> return src;
>
> switch (a) {
> case GLSL_TYPE_UINT:
> case GLSL_TYPE_INT:
> - if (b == GLSL_TYPE_FLOAT)
> + switch(b) {
> + case GLSL_TYPE_UINT:
> + result = new(ctx) ir_expression(ir_unop_u2i, desired_type, src, NULL);
> + break;
> + case GLSL_TYPE_INT:
> + result = new(ctx) ir_expression(ir_unop_i2u, desired_type, src, NULL);
> + break;
> + case GLSL_TYPE_FLOAT:
> result = new(ctx) ir_expression(ir_unop_f2i, desired_type, src, NULL);
With the other changes, leaving this as-is is kind of hinkey. Here the
result base type will be GLSL_TYPE_INT, but the desired based type could
be GLSL_TYPE_UINT.
> - else {
> - assert(b == GLSL_TYPE_BOOL);
> + break;
> + case GLSL_TYPE_BOOL:
> result = new(ctx) ir_expression(ir_unop_b2i, desired_type, src, NULL);
> + break;
> }
> break;
> case GLSL_TYPE_FLOAT:
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index a3623b3..85c54ba 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -272,6 +272,7 @@ ir_expression::ir_expression(int op, ir_rvalue *op0)
>
> case ir_unop_f2i:
> case ir_unop_b2i:
> + case ir_unop_u2i:
> this->type = glsl_type::get_instance(GLSL_TYPE_INT,
> op0->type->vector_elements, 1);
> break;
> @@ -288,6 +289,11 @@ ir_expression::ir_expression(int op, ir_rvalue *op0)
> this->type = glsl_type::get_instance(GLSL_TYPE_BOOL,
> op0->type->vector_elements, 1);
> break;
> +
> + case ir_unop_i2u:
> + this->type = glsl_type::get_instance(GLSL_TYPE_UINT,
> + op0->type->vector_elements, 1);
> + break;
>
> case ir_unop_noise:
> this->type = glsl_type::float_type;
> @@ -419,6 +425,8 @@ static const char *const operator_strs[] = {
> "i2b",
> "b2i",
> "u2f",
> + "i2u",
> + "u2i",
> "any",
> "trunc",
> "ceil",
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index a419843..a9de530 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -789,6 +789,8 @@ enum ir_expression_operation {
> ir_unop_i2b, /**< int-to-boolean conversion */
> ir_unop_b2i, /**< Boolean-to-int conversion */
> ir_unop_u2f, /**< Unsigned-to-float conversion. */
> + ir_unop_i2u, /**< Integer-to-unsigned conversion. */
> + ir_unop_u2i, /**< Unsigned-to-integer conversion. */
> ir_unop_any,
>
> /**
> diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp
> index 2a30848..341c2e6 100644
> --- a/src/glsl/ir_constant_expression.cpp
> +++ b/src/glsl/ir_constant_expression.cpp
> @@ -166,6 +166,14 @@ ir_expression::constant_expression_value()
> data.b[c] = op[0]->value.u[c] ? true : false;
> }
> break;
> + case ir_unop_u2i:
> + assert(op[0]->type->base_type == GLSL_TYPE_UINT);
> + /* Conversion from uint to int is a no-op. */
> + break;
> + case ir_unop_i2u:
> + assert(op[0]->type->base_type == GLSL_TYPE_INT);
> + /* Type conversion from int to uint is a no-op. */
> + break;
>
> case ir_unop_any:
> assert(op[0]->type->is_boolean());
> diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
> index ec79d05..f3fceb2 100644
> --- a/src/glsl/ir_validate.cpp
> +++ b/src/glsl/ir_validate.cpp
> @@ -280,6 +280,14 @@ ir_validate::visit_leave(ir_expression *ir)
> assert(ir->operands[0]->type->base_type == GLSL_TYPE_UINT);
> assert(ir->type->base_type == GLSL_TYPE_FLOAT);
> break;
> + case ir_unop_i2u:
> + assert(ir->operands[0]->type->base_type == GLSL_TYPE_INT);
> + assert(ir->type->base_type == GLSL_TYPE_UINT);
> + break;
> + case ir_unop_u2i:
> + assert(ir->operands[0]->type->base_type == GLSL_TYPE_UINT);
> + assert(ir->type->base_type == GLSL_TYPE_INT);
> + break;
>
> case ir_unop_any:
> assert(ir->operands[0]->type->base_type == GLSL_TYPE_BOOL);
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk2syxYACgkQX1gOwKyEAw9GuQCaA4iferLtXbj1wPTLuWnoqItf
Uw8AmQHUUE/caOiTO/JBdpeshouFV4oo
=7fCC
-----END PGP SIGNATURE-----
More information about the mesa-dev
mailing list