subject:"\[Mesa\-dev\] \[PATCH 1\/3\] glsl\: correctly handle conversions between int and uint"

Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

2011-04-19 Thread Ian Romanick

-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

2011-04-19 Thread Bryan Cain

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 mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

2011-04-18 Thread Bryan Cain

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

2011-04-18 Thread Kenneth Graunke


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

2011-04-18 Thread Ian Romanick

-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

Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

2011-04-18 Thread Bryan Cain

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

Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

[Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

Re: [Mesa-dev] [PATCH 1/3] glsl: correctly handle conversions between int and uint

6 matches

Site Navigation

Mail list logo

Footer information