[Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
>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.
---
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);
- 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);
--
1.7.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
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.
Hmm. I'm not sure about this patch.
First of all, the comment above convert_component explicitly states that
it implements the rules in section 5.4.1, "Conversion and Scalar
Constructors", not section 4.1.10's "Implicit Conversions." For
historical reasons, there are two sets of rules (yes, it's confusing!).
The "constructor rules" do seem to allow int/uint conversions. For
example, I believe these are allowed:
uvec4(1, 2, 3, 4)
ivec4(1u, 2u, 3, 4)
However, the implicit conversion rules do not:
2u + 3
is definitely not allowed.
We definitely need to do both kinds of semantic checking.
A separate question is whether we need explicit i2u and u2i operations
in the IR. I think i2u and u2i would be no-ops in almost every case
since it's the same bits...just whether you interpret them as having a
sign bit or not. I think the idea was to just change the type:
(expression uint + (constant int (0)) (constant int (1)))
This is also why we don't have f2u, b2u, and u2b - they're the same for
int and uint. The one operation we do have, u2f, is because it does
matter whether the source operand is interpreted as unsigned or not.
Still, this is awfully non-symmetric, so it may be worth having them...
I'll defer to Ian here. If we do add patch 1/3, 3/3 looks good.
Thanks,
--Kenneth
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
-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;
>
Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
This patch was revised several times before submitting, and even after
that I still wasn't sure that adding conversion operations was the right
solution. So I'm not really surprised that there are problems with this
one. :)
On 04/18/2011 06:36 PM, Ian Romanick wrote:
> 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?
Yes, usages like this:
int var1 = 7;
uint var2 = uint(var1);
They were causing a type mismatch error in ast_to_hir.cpp because
convert_component() was treating the uint(int) constructor as a no-op.
>
> > ---
> > 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.
Unless I'm missing something, that case is handled by the line "if (a ==
b)".
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 04/18/2011 04:51 PM, Bryan Cain wrote:
> This patch was revised several times before submitting, and even after
> that I still wasn't sure that adding conversion operations was the right
> solution. So I'm not really surprised that there are problems with this
> one. :)
>
> On 04/18/2011 06:36 PM, Ian Romanick wrote:
>> 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?
> Yes, usages like this:
>
> int var1 = 7;
> uint var2 = uint(var1);
>
> They were causing a type mismatch error in ast_to_hir.cpp because
> convert_component() was treating the uint(int) constructor as a no-op.
Ah, that makes sense. I just sent some patches to the piglit mailing
list with some tests in this vein.
You've convinced me that we need i2u and u2i unary operators. I don't
think we need b2u and u2b operators because we can just do (i2b(u2i
value)) and (i2u(b2i value)). Ditto for f2i. This should result in
less churn in the backends since u2i and i2u will universally be no-ops.
Almost every place that checks is_integer() will need to check for the
specific type, either GLSL_TYPE_UINT or GLSL_TYPE_INT, that it expects.
This means that your patch 2/3, which is already applied, will need to
be reverted.
>>> ---
>>> 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.
> Unless I'm missing something, that case is handled by the line "if (a ==
> b)".
What I meant was uint(float) and int(float) will both generate an
ir_unop_f2i, which has type int. So, 'uint x = uint(5.2);' will result
in a type error. Right?
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk2uEBkACgkQX1gOwKyEAw8esQCdEawL5QiLogZuqLrstfyI7Dz9
ub0AoJuTNzMseA2K3vdrtSzjEQ8sLctO
=yFr3
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint
On 04/19/2011 05:43 PM, Ian Romanick wrote:
> On 04/18/2011 04:51 PM, Bryan Cain wrote:
> > This patch was revised several times before submitting, and even after
> > that I still wasn't sure that adding conversion operations was the right
> > solution. So I'm not really surprised that there are problems with this
> > one. :)
>
> > On 04/18/2011 06:36 PM, Ian Romanick wrote:
> >> 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?
> > Yes, usages like this:
>
> > int var1 = 7;
> > uint var2 = uint(var1);
>
> > They were causing a type mismatch error in ast_to_hir.cpp because
> > convert_component() was treating the uint(int) constructor as a no-op.
>
> Ah, that makes sense. I just sent some patches to the piglit mailing
> list with some tests in this vein.
>
> You've convinced me that we need i2u and u2i unary operators. I don't
> think we need b2u and u2b operators because we can just do (i2b(u2i
> value)) and (i2u(b2i value)). Ditto for f2i. This should result in
> less churn in the backends since u2i and i2u will universally be no-ops.
>
> Almost every place that checks is_integer() will need to check for the
> specific type, either GLSL_TYPE_UINT or GLSL_TYPE_INT, that it expects.
> This means that your patch 2/3, which is already applied, will need to
> be reverted.
Wouldn't it only apply to checks for destination types of most
operations, not source types?
In fact, if instructions store the destination type, what cases would
require the compiler beyond ast_to_hir to even care about the difference
between int and uint?
>
> >>> ---
> >>> 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.
> > Unless I'm missing something, that case is handled by the line "if (a ==
> > b)".
>
> What I meant was uint(float) and int(float) will both generate an
> ir_unop_f2i, which has type int. So, 'uint x = uint(5.2);' will result
> in a type error. Right?
It's not resulting in one for me with the patch applied. Are the exact
source and destination types not stored as part of an expression in GLSL IR?
___
mesa-dev
