[Mesa-dev] [PATCH 11/22] glsl: Add ir_binop_mul to get_range

[Bruno Jimenez]

On Sat, 2015-01-03 at 20:18 +0100, Thomas Helland wrote:
> V2: Add some air for readability
>     Use the new IS_CONSTANT macro
>     Combine if-blocks for reduced code-duplication
>     Split out into separate function for reuse later
> ---
>  src/glsl/opt_minmax.cpp | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
> index 4153a48..e4aa677 100644
> --- a/src/glsl/opt_minmax.cpp
> +++ b/src/glsl/opt_minmax.cpp
> @@ -300,6 +300,34 @@ resolv_add_range(minmax_range r0, minmax_range r1)
>     return minmax_range(low, high);
>  }
>  
> +/* Takes the range of the operands in a mul-operation as parameters
> + * and uses this to solve the range of the mul-operation itself.
> + */
> +static minmax_range
> +resolv_mul_range(minmax_range r0, minmax_range r1)
> +{
> +   ir_constant *low = NULL;
> +   ir_constant *high = NULL;
> +
> +   // Both are positive, or both are negative, result is positive
> +   if ((IS_CONSTANT(r0.low, >=, 0.0f) && IS_CONSTANT(r1.low, >=, 0.0f)) ||
> +       (IS_CONSTANT(r0.high, <=, 0.0f) && IS_CONSTANT(r1.high, <=, 0.0f))) {
> +      low = mul(r0.low, r1.low)->constant_expression_value();
> +      if (r0.high && r1.high)
> +         high = mul(r0.high, r1.high)->constant_expression_value();
> +   }

Hi,

I think that this one is wrong: imagine the case where r0.low,high and
r1.low,high are negative, for example [-2,-1] [-4,-3].

In this case, the resulting range for the multiplication is bound in [3,
8] = [high*high, low*low], not [low*low, high*high].

- Bruno

> +
> +   // r0 pos and r1 neg, or r0 neg and r1 pos, result is negative
> +   if ((IS_CONSTANT(r0.low, >=, 0.0f) && IS_CONSTANT(r1.high, <=, 0.0f)) ||
> +       (IS_CONSTANT(r1.low, >=, 0.0f) && IS_CONSTANT(r0.high, <=, 0.0f))) {
> +      high = mul(r0.low, r1.high)->constant_expression_value();
> +      if (r0.high && r1.low)
> +         low = mul(r0.high, r1.low)->constant_expression_value();
> +   }
> +
> +   return minmax_range(low, high);
> +}
> +
>  static minmax_range
>  get_range(ir_rvalue *rval)
>  {
> @@ -389,6 +417,11 @@ get_range(ir_rvalue *rval)
>           r1 = get_range(expr->operands[1]);
>           return combine_range(r0, r1, expr->operation == ir_binop_min);
>  
> +      case ir_binop_mul:
> +         r0 = get_range(expr->operands[0]);
> +         r1 = get_range(expr->operands[1]);
> +         return resolv_mul_range(r0, r1);
> +
>        default:
>           break;
>        }