[Mesa-dev] [PATCH 2/8] glsl: tidy up get_num_operands()

2017-08-08 Thread Timothy Arceri 
Also add a comment that this should only be used by the ir_reader
interface for testing purposes.

v2:
 - fix grammar in comment
 - use unreachable rather than assert

Reviewed-by: Thomas Helland 
---
 src/compiler/glsl/ir.cpp |  9 ++---
 src/compiler/glsl/ir.h   | 14 +++---
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 51b875058b..98bbd91539 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -549,39 +549,42 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, 
ir_rvalue *op1,
case ir_triop_csel:
   this->type = op1->type;
   break;
 
default:
   assert(!"not reached: missing automatic type setup for ir_expression");
   this->type = glsl_type::float_type;
}
 }
 
-unsigned int
+/**
+ * This is only here for ir_reader to used for testing purposes. Please use
+ * the precomputed num_operands field if you need the number of operands.
+ */
+unsigned
 ir_expression::get_num_operands(ir_expression_operation op)
 {
assert(op <= ir_last_opcode);
 
if (op <= ir_last_unop)
   return 1;
 
if (op <= ir_last_binop)
   return 2;
 
if (op <= ir_last_triop)
   return 3;
 
if (op <= ir_last_quadop)
   return 4;
 
-   assert(false);
-   return 0;
+   unreachable("Could not calculate number of operands");
 }
 
 #include "ir_expression_operation_strings.h"
 
 const char*
 depth_layout_string(ir_depth_layout layout)
 {
switch(layout) {
case ir_depth_layout_none:  return "";
case ir_depth_layout_any:   return "depth_any";
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index 58e6356566..d53b44304d 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -1531,32 +1531,24 @@ public:
 * The "variable_context" hash table links ir_variable * to ir_constant *
 * that represent the variables' values.  \c NULL represents an empty
 * context.
 *
 * If the expression cannot be constant folded, this method will return
 * \c NULL.
 */
virtual ir_constant *constant_expression_value(struct hash_table 
*variable_context = NULL);
 
/**
-* Determine the number of operands used by an expression
+* This is only here for ir_reader to used for testing purposes please use
+* the precomputed num_operands field if you need the number of operands.
 */
-   static unsigned int get_num_operands(ir_expression_operation);
-
-   /**
-* Determine the number of operands used by an expression
-*/
-   unsigned int get_num_operands() const
-   {
-  return (this->operation == ir_quadop_vector)
-? this->type->vector_elements : get_num_operands(operation);
-   }
+   static unsigned get_num_operands(ir_expression_operation);
 
/**
 * Return whether the expression operates on vectors horizontally.
 */
bool is_horizontal() const
{
   return operation == ir_binop_all_equal ||
  operation == ir_binop_any_nequal ||
  operation == ir_binop_dot ||
  operation == ir_binop_vector_extract ||
-- 
2.13.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] glsl: tidy up get_num_operands()

2017-08-08 Thread Thomas Helland 
2017-08-07 2:18 GMT+00:00 Timothy Arceri :
> Also add a comment that this should only be used by the ir_reader
> interface for testing purposes.
> ---
>  src/compiler/glsl/ir.cpp |  8 ++--
>  src/compiler/glsl/ir.h   | 14 +++---
>  2 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index d501e19c01..194d535790 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -549,38 +549,42 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, 
> ir_rvalue *op1,
> case ir_triop_csel:
>this->type = op1->type;
>break;
>
> default:
>assert(!"not reached: missing automatic type setup for ir_expression");
>this->type = glsl_type::float_type;
> }
>  }
>
> -unsigned int
> +/**
> + * This is only here for ir_reader to used for testing purposes please use
> + * the precomputed num_operands field if you need the number of operands.
> + */

 purposes. Please use .

> +unsigned
>  ir_expression::get_num_operands(ir_expression_operation op)
>  {
> assert(op <= ir_last_opcode);
>
> if (op <= ir_last_unop)
>return 1;
>
> if (op <= ir_last_binop)
>return 2;
>
> if (op <= ir_last_triop)
>return 3;
>
> if (op <= ir_last_quadop)
>return 4;
>
> -   assert(false);
> +   assert(!"could not calculate number of operands");

Can we do:
unreachable("Could not calculate number of operands");

The patch is:
Reviewed-by: Thomas Helland

> return 0;
>  }
>
>  #include "ir_expression_operation_strings.h"
>
>  const char*
>  depth_layout_string(ir_depth_layout layout)
>  {
> switch(layout) {
> case ir_depth_layout_none:  return "";
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index 377e03657d..70b5716965 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -1531,32 +1531,24 @@ public:
>  * The "variable_context" hash table links ir_variable * to ir_constant *
>  * that represent the variables' values.  \c NULL represents an empty
>  * context.
>  *
>  * If the expression cannot be constant folded, this method will return
>  * \c NULL.
>  */
> virtual ir_constant *constant_expression_value(struct hash_table 
> *variable_context = NULL);
>
> /**
> -* Determine the number of operands used by an expression
> +* This is only here for ir_reader to used for testing purposes please use
> +* the precomputed num_operands field if you need the number of operands.
>  */
> -   static unsigned int get_num_operands(ir_expression_operation);
> -
> -   /**
> -* Determine the number of operands used by an expression
> -*/
> -   unsigned int get_num_operands() const
> -   {
> -  return (this->operation == ir_quadop_vector)
> -? this->type->vector_elements : get_num_operands(operation);
> -   }
> +   static unsigned get_num_operands(ir_expression_operation);
>
> /**
>  * Return whether the expression operates on vectors horizontally.
>  */
> bool is_horizontal() const
> {
>return operation == ir_binop_all_equal ||
>   operation == ir_binop_any_nequal ||
>   operation == ir_binop_dot ||
>   operation == ir_binop_vector_extract ||
> --
> 2.13.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/8] glsl: tidy up get_num_operands()

2017-08-06 Thread Timothy Arceri 
Also add a comment that this should only be used by the ir_reader
interface for testing purposes.
---
 src/compiler/glsl/ir.cpp |  8 ++--
 src/compiler/glsl/ir.h   | 14 +++---
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index d501e19c01..194d535790 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -549,38 +549,42 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, 
ir_rvalue *op1,
case ir_triop_csel:
   this->type = op1->type;
   break;
 
default:
   assert(!"not reached: missing automatic type setup for ir_expression");
   this->type = glsl_type::float_type;
}
 }
 
-unsigned int
+/**
+ * This is only here for ir_reader to used for testing purposes please use
+ * the precomputed num_operands field if you need the number of operands.
+ */
+unsigned
 ir_expression::get_num_operands(ir_expression_operation op)
 {
assert(op <= ir_last_opcode);
 
if (op <= ir_last_unop)
   return 1;
 
if (op <= ir_last_binop)
   return 2;
 
if (op <= ir_last_triop)
   return 3;
 
if (op <= ir_last_quadop)
   return 4;
 
-   assert(false);
+   assert(!"could not calculate number of operands");
return 0;
 }
 
 #include "ir_expression_operation_strings.h"
 
 const char*
 depth_layout_string(ir_depth_layout layout)
 {
switch(layout) {
case ir_depth_layout_none:  return "";
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index 377e03657d..70b5716965 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -1531,32 +1531,24 @@ public:
 * The "variable_context" hash table links ir_variable * to ir_constant *
 * that represent the variables' values.  \c NULL represents an empty
 * context.
 *
 * If the expression cannot be constant folded, this method will return
 * \c NULL.
 */
virtual ir_constant *constant_expression_value(struct hash_table 
*variable_context = NULL);
 
/**
-* Determine the number of operands used by an expression
+* This is only here for ir_reader to used for testing purposes please use
+* the precomputed num_operands field if you need the number of operands.
 */
-   static unsigned int get_num_operands(ir_expression_operation);
-
-   /**
-* Determine the number of operands used by an expression
-*/
-   unsigned int get_num_operands() const
-   {
-  return (this->operation == ir_quadop_vector)
-? this->type->vector_elements : get_num_operands(operation);
-   }
+   static unsigned get_num_operands(ir_expression_operation);
 
/**
 * Return whether the expression operates on vectors horizontally.
 */
bool is_horizontal() const
{
   return operation == ir_binop_all_equal ||
  operation == ir_binop_any_nequal ||
  operation == ir_binop_dot ||
  operation == ir_binop_vector_extract ||
-- 
2.13.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev