[Mesa-dev] [v2 PATCH 09/16] glsl: Optimize clamp(x, 0, 1) as saturate(x)

[Erik Faye-Lund]

On Mon, Jul 7, 2014 at 7:18 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Mon, Jul 7, 2014 at 6:57 AM, Abdiel Janulgue
> <abdiel.janulgue at linux.intel.com> wrote:
>> v2: Check that the base type is float (Ian Romanick)
>>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
>> ---
>>  src/glsl/opt_algebraic.cpp | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
>> index ac7514a..2ad561c 100644
>> --- a/src/glsl/opt_algebraic.cpp
>> +++ b/src/glsl/opt_algebraic.cpp
>> @@ -614,6 +614,40 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
>>
>>        break;
>>
>> +   case ir_binop_min:
>> +   case ir_binop_max:
>> +      if (ir->type->base_type != GLSL_TYPE_FLOAT)
>> +         break;
>> +      /* Replace min(max) operations and its commutative combinations with
>> +       * a saturate operation
>> +       */
>> +      for (int op = 0; op < 2; op++) {
>> +         ir_expression *minmax = op_expr[op];
>> +         ir_constant *outer_const = op_const[(op == 0) ? 1 : 0];
>
> We use (1 - op) in other places in opt_algebraic. (Same comment below)
>
>> +         ir_expression_operation op_cond = (ir->operation == ir_binop_max) ?
>> +            ir_binop_min : ir_binop_max;
>> +
>> +         if (!minmax || !outer_const || (minmax->operation != op_cond))
>> +            continue;
>> +
>> +         /* Found a min/max operation. Now try to see if its operands
>> +          * meet our conditions that we can do just a single saturate operation
>> +          */
>> +         for (int minmax_op = 0; minmax_op < 2; minmax_op++) {
>> +            ir_rvalue *inner_val_a = minmax->operands[minmax_op];
>> +            ir_rvalue *inner_val_b = minmax->operands[(minmax_op == 0) ? 1 : 0];
>> +
>> +            if (!inner_val_a || !inner_val_b)
>> +               continue;
>> +
>> +            /* Found a min (max(x, 0), 1.0) */
>
> This comment tripped me up for a second. This really means that you've
> found either
>
>   - min(max(x, 0.0), 1.0); or
>   - max(min(x, 1.0), 0.0)

Hmm, but are optimizing both of these to saturate OK? Shouldn't
min(max(NaN, 0.0), 1.0) give 0.0, whereas max(min(NaN, 1.0), 0.0) give
1.0?