[Mesa-dev] [PATCH 03/16] glsl: Move common code to ir_constant_util.h

[Ian Romanick]

On 11/16/2014 05:51 PM, Thomas Helland wrote:
> This will allow for less code duplication.
> I'll be using this in opt_minmax in the coming commits.
> ---
>  src/glsl/ir_constant_util.h | 122 ++++++++++++++++++++++++++++++++++++++++++++
>  src/glsl/opt_algebraic.cpp  |  88 +-------------------------------
>  src/glsl/opt_minmax.cpp     |  17 +-----
>  3 files changed, 124 insertions(+), 103 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..4e0fede
> --- /dev/null
> +++ b/src/glsl/ir_constant_util.h
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright © 2010 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * \file ir_constant_util.h
> + *
> + * A collection of utility functions for use on constants
> + * Shamelessly cut-pasted from opt_algebraic, etc
> + *
> + * Author: Thomas Helland <thomashelland90 at gmail.com>

We don't usually put author tags in comments any more.  git-log does
that for us. :)

> + */
> +
> +#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;

I don't think putting 'using namespace' in a header file is a good idea.
 That could cause surprising results in files that include it.

> +
> +/* 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_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 af1f544..8c2f15c 100644
> --- a/src/glsl/opt_algebraic.cpp
> +++ b/src/glsl/opt_algebraic.cpp
> @@ -29,13 +29,10 @@
>   */
>  
>  #include "ir.h"
> -#include "ir_visitor.h"
>  #include "ir_rvalue_visitor.h"
>  #include "ir_optimization.h"
> -#include "ir_builder.h"
>  #include "glsl_types.h"
> -
> -using namespace ir_builder;
> +#include "ir_constant_util.h"
>  
>  namespace {
>  
> @@ -68,8 +65,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,72 +75,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_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)
>  {
> @@ -264,21 +193,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..89970d5 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 {
>  
> @@ -429,16 +424,6 @@ ir_minmax_visitor::prune_expression(ir_expression *expr, minmax_range baserange)
>     return expr;
>  }
>  
> -static ir_rvalue *
> -swizzle_if_required(ir_expression *expr, ir_rvalue *rval)
> -{
> -   if (expr->type->is_vector() && rval->type->is_scalar()) {
> -      return swizzle(rval, SWIZZLE_XXXX, expr->type->vector_elements);
> -   } else {
> -      return rval;
> -   }
> -}
> -
>  void
>  ir_minmax_visitor::handle_rvalue(ir_rvalue **rvalue)
>  {
>