[Mesa-dev] [PATCH 1/6] glsl: Move common code to constant_util

[Matt Turner]

On Wed, Oct 29, 2014 at 6:11 PM, Thomas Helland
<thomashelland90 at gmail.com> wrote:
> This will be used later on in opt_minmax
>
> Signed-off-by: Thomas Helland <thomashelland90 at gmail.com>
> ---
>  src/glsl/ir_constant_util.h | 103 ++++++++++++++++++++++++++++++++++++++++++++
>  src/glsl/opt_algebraic.cpp  |  95 ++--------------------------------------
>  src/glsl/opt_minmax.cpp     |  19 ++------
>  3 files changed, 109 insertions(+), 108 deletions(-)
>  create mode 100644 src/glsl/ir_constant_util.h
>
> diff --git a/src/glsl/ir_constant_util.h b/src/glsl/ir_constant_util.h
> new file mode 100644
> index 0000000..b3b9a19
> --- /dev/null
> +++ b/src/glsl/ir_constant_util.h
> @@ -0,0 +1,103 @@
> +/*
> + * ir_constant_util.h
> + *
> + *  Created on: 13. okt. 2014
> + *      Author: helland

The file needs to have a copyright and license header.

We've been trying to get away from author tags, but if you want one,
at least put your whole name and email like the others. I don't see
any value in "Created on"

> + */
> +
> +#ifndef IR_CONSTANT_UTIL_H_
> +#define IR_CONSTANT_UTIL_H_
> +
> +#include "main/macros.h"
> +#include "ir_builder.h"
> +#include "program/prog_instruction.h"
> +
> +using namespace ir_builder;
> +
> +/* When eliminating an expression and just returning one of its operands,
> + * we may need to swizzle that operand out to a vector if the expression was
> + * vector type.
> + */
> +static ir_rvalue *
> +swizzle_if_required(ir_expression *expr,
> +                 ir_rvalue *operand)
> +{
> +   if (expr->type->is_vector() && operand->type->is_scalar()) {
> +      return swizzle(operand, SWIZZLE_XXXX, expr->type->vector_elements);
> +   } else
> +      return operand;
> +}
> +
> +static inline bool
> +is_vec_zero(ir_constant *ir)
> +{
> +   return (ir == NULL) ? false : ir->is_zero();
> +}
> +
> +static inline bool
> +is_vec_one(ir_constant *ir)
> +{
> +   return (ir == NULL) ? false : ir->is_one();
> +}
> +
> +static inline bool
> +is_vec_two(ir_constant *ir)
> +{
> +   return (ir == NULL) ? false : ir->is_value(2.0, 2);
> +}
> +
> +static inline bool
> +is_vec_negative_one(ir_constant *ir)
> +{
> +   return (ir == NULL) ? false : ir->is_negative_one();
> +}
> +
> +static inline bool
> +is_vec_basis(ir_constant *ir)
> +{
> +   return (ir == NULL) ? false : ir->is_basis();
> +}
> +
> +static inline bool
> +is_valid_vec_const(ir_constant *ir)
> +{
> +   if (ir == NULL)
> +      return false;
> +
> +   if (!ir->type->is_scalar() && !ir->type->is_vector())
> +      return false;
> +
> +   return true;
> +}
> +
> +static inline bool
> +is_less_than_one(ir_constant *ir)
> +{
> +   if (!is_valid_vec_const(ir))
> +      return false;
> +
> +   unsigned component = 0;
> +   for (int c = 0; c < ir->type->vector_elements; c++) {
> +      if (ir->get_float_component(c) < 1.0f)
> +         component++;
> +   }
> +
> +   return (component == ir->type->vector_elements);
> +}
> +
> +static inline bool
> +is_greater_than_zero(ir_constant *ir)
> +{
> +   if (!is_valid_vec_const(ir))
> +      return false;
> +
> +   unsigned component = 0;
> +   for (int c = 0; c < ir->type->vector_elements; c++) {
> +      if (ir->get_float_component(c) > 0.0f)
> +         component++;
> +   }
> +
> +   return (component == ir->type->vector_elements);
> +}
> +
> +#endif /* IR_CONSTANT_UTIL_H_ */
> diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
> index 0cdb8ec..8392017 100644
> --- a/src/glsl/opt_algebraic.cpp
> +++ b/src/glsl/opt_algebraic.cpp
> @@ -29,13 +29,13 @@
>   */
>
>  #include "ir.h"
> -#include "ir_visitor.h"
> +//#include "ir_visitor.h"

Presumably you meant to just delete this line.

>  #include "ir_rvalue_visitor.h"
>  #include "ir_optimization.h"
> -#include "ir_builder.h"
>  #include "glsl_types.h"
> +#include "ir_constant_util.h"
> +

Extra new line.

>
> -using namespace ir_builder;
>
>  namespace {
>
> @@ -68,8 +68,6 @@ public:
>                              int op1,
>                              ir_expression *ir2,
>                              int op2);
> -   ir_rvalue *swizzle_if_required(ir_expression *expr,
> -                                 ir_rvalue *operand);
>
>     const struct gl_shader_compiler_options *options;
>     void *mem_ctx;
> @@ -80,78 +78,6 @@ public:
>
>  } /* unnamed namespace */
>
> -static inline bool
> -is_vec_zero(ir_constant *ir)
> -{
> -   return (ir == NULL) ? false : ir->is_zero();
> -}
> -
> -static inline bool
> -is_vec_one(ir_constant *ir)
> -{
> -   return (ir == NULL) ? false : ir->is_one();
> -}
> -
> -static inline bool
> -is_vec_two(ir_constant *ir)
> -{
> -   return (ir == NULL) ? false : ir->is_value(2.0, 2);
> -}
> -
> -static inline bool
> -is_vec_negative_one(ir_constant *ir)
> -{
> -   return (ir == NULL) ? false : ir->is_negative_one();
> -}
> -
> -static inline bool
> -is_vec_basis(ir_constant *ir)
> -{
> -   return (ir == NULL) ? false : ir->is_basis();
> -}
> -
> -static inline bool
> -is_valid_vec_const(ir_constant *ir)
> -{
> -   if (ir == NULL)
> -      return false;
> -
> -   if (!ir->type->is_scalar() && !ir->type->is_vector())
> -      return false;
> -
> -   return true;
> -}
> -
> -static inline bool
> -is_less_than_one(ir_constant *ir)
> -{
> -   if (!is_valid_vec_const(ir))
> -      return false;
> -
> -   unsigned component = 0;
> -   for (int c = 0; c < ir->type->vector_elements; c++) {
> -      if (ir->get_float_component(c) < 1.0f)
> -         component++;
> -   }
> -
> -   return (component == ir->type->vector_elements);
> -}
> -
> -static inline bool
> -is_greater_than_zero(ir_constant *ir)
> -{
> -   if (!is_valid_vec_const(ir))
> -      return false;
> -
> -   unsigned component = 0;
> -   for (int c = 0; c < ir->type->vector_elements; c++) {
> -      if (ir->get_float_component(c) > 0.0f)
> -         component++;
> -   }
> -
> -   return (component == ir->type->vector_elements);
> -}
> -
>  static void
>  update_type(ir_expression *ir)
>  {
> @@ -270,21 +196,6 @@ ir_algebraic_visitor::reassociate_constant(ir_expression *ir1, int const_index,
>     return false;
>  }
>
> -/* When eliminating an expression and just returning one of its operands,
> - * we may need to swizzle that operand out to a vector if the expression was
> - * vector type.
> - */
> -ir_rvalue *
> -ir_algebraic_visitor::swizzle_if_required(ir_expression *expr,
> -                                         ir_rvalue *operand)
> -{
> -   if (expr->type->is_vector() && operand->type->is_scalar()) {
> -      return new(mem_ctx) ir_swizzle(operand, 0, 0, 0, 0,
> -                                    expr->type->vector_elements);
> -   } else
> -      return operand;
> -}
> -
>  ir_rvalue *
>  ir_algebraic_visitor::handle_expression(ir_expression *ir)
>  {
> diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
> index 32fb2d7..e4141bc 100644
> --- a/src/glsl/opt_minmax.cpp
> +++ b/src/glsl/opt_minmax.cpp
> @@ -31,15 +31,10 @@
>   */
>
>  #include "ir.h"
> -#include "ir_visitor.h"
>  #include "ir_rvalue_visitor.h"
>  #include "ir_optimization.h"
> -#include "ir_builder.h"
> -#include "program/prog_instruction.h"
>  #include "glsl_types.h"
> -#include "main/macros.h"
> -
> -using namespace ir_builder;
> +#include "ir_constant_util.h"
>
>  namespace {
>
> @@ -215,6 +210,7 @@ larger_constant(ir_constant *a, ir_constant *b)
>        return a;
>  }
>
> +

Extra new line.

>  /* Combines two ranges by doing an element-wise min() / max() depending on the
>   * operation.
>   */
> @@ -288,6 +284,7 @@ get_range(ir_rvalue *rval)
>     return minmax_range();
>  }
>
> +

Again.

The intention of the patch seems good.