[Mesa-dev] [PATCH] glsl: Rewrite and fix min/max to saturate optimization.

[Matt Turner]

There were some bugs, and the code was really difficult to follow. We
would optimize

   min(max(x, b), 1.0) into max(sat(x), b)

but not pay attention to the order of min/max and also do

   max(min(x, b), 1.0) into max(sat(x), b)

Corrects four shaders from Champions of Regnum that do

   min(max(x, 1), 10)

Cc: "10.5" <mesa-stable at lists.freedesktop.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89180
---
 src/glsl/opt_algebraic.cpp | 75 ++++++++++++++++++++++++++++------------------
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
index 6784242..c3f3842 100644
--- a/src/glsl/opt_algebraic.cpp
+++ b/src/glsl/opt_algebraic.cpp
@@ -744,48 +744,65 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
        * a saturate operation
        */
       for (int op = 0; op < 2; op++) {
-         ir_expression *minmax = op_expr[op];
+         ir_expression *inner_expr = op_expr[op];
          ir_constant *outer_const = op_const[1 - op];
          ir_expression_operation op_cond = (ir->operation == ir_binop_max) ?
             ir_binop_min : ir_binop_max;
 
-         if (!minmax || !outer_const || (minmax->operation != op_cond))
+         if (!inner_expr || !outer_const || (inner_expr->operation != op_cond))
             continue;
 
+         /* One of these has to be a constant */
+         if (!inner_expr->operands[0]->as_constant() &&
+             !inner_expr->operands[1]->as_constant())
+            break;
+
          /* Found a min(max) combination. 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[1 - minmax_op];
+            ir_rvalue *x = inner_expr->operands[minmax_op];
+            ir_rvalue *y = inner_expr->operands[1 - minmax_op];
 
-            if (!inner_val_a || !inner_val_b)
+            ir_constant *inner_const = y->as_constant();
+            if (!inner_const)
                continue;
 
-            /* Found a {min|max} ({max|min} (x, 0.0), 1.0) operation and its variations */
-            if ((outer_const->is_one() && inner_val_a->is_zero()) ||
-                (inner_val_a->is_one() && outer_const->is_zero()))
-               return saturate(inner_val_b);
-
-            /* Found a {min|max} ({max|min} (x, 0.0), b) where b < 1.0
-             * and its variations
-             */
-            if (is_less_than_one(outer_const) && inner_val_b->is_zero())
-               return expr(ir_binop_min, saturate(inner_val_a), outer_const);
-
-            if (!inner_val_b->as_constant())
-               continue;
-
-            if (is_less_than_one(inner_val_b->as_constant()) && outer_const->is_zero())
-               return expr(ir_binop_min, saturate(inner_val_a), inner_val_b);
-
-            /* Found a {min|max} ({max|min} (x, b), 1.0), where b > 0.0
-             * and its variations
-             */
-            if (outer_const->is_one() && is_greater_than_zero(inner_val_b->as_constant()))
-               return expr(ir_binop_max, saturate(inner_val_a), inner_val_b);
-            if (inner_val_b->as_constant()->is_one() && is_greater_than_zero(outer_const))
-               return expr(ir_binop_max, saturate(inner_val_a), outer_const);
+            /* min(max(x, 0.0), 1.0) is sat(x) */
+            if (ir->operation == ir_binop_min &&
+                inner_const->is_zero() &&
+                outer_const->is_one())
+               return saturate(x);
+
+            /* max(min(x, 1.0), 0.0) is sat(x) */
+            if (ir->operation == ir_binop_max &&
+                inner_const->is_one() &&
+                outer_const->is_zero())
+               return saturate(x);
+
+            /* min(max(x, 0.0), b) where b < 1.0 is sat(min(x, b)) */
+            if (ir->operation == ir_binop_min &&
+                inner_const->is_zero() &&
+                is_less_than_one(outer_const))
+               return saturate(expr(ir_binop_min, x, outer_const));
+
+            /* max(min(x, b), 0.0) where b < 1.0 is sat(min(x, b)) */
+            if (ir->operation == ir_binop_max &&
+                is_less_than_one(inner_const) &&
+                outer_const->is_zero())
+               return saturate(expr(ir_binop_min, x, inner_const));
+
+            /* max(min(x, 1.0), b) where b > 0.0 is sat(max(x, b)) */
+            if (ir->operation == ir_binop_max &&
+                inner_const->is_one() &&
+                is_greater_than_zero(outer_const))
+               return saturate(expr(ir_binop_max, x, outer_const));
+
+            /* min(max(x, b), 1.0) where b > 0.0 is sat(max(x, b)) */
+            if (ir->operation == ir_binop_min &&
+                is_greater_than_zero(inner_const) &&
+                outer_const->is_one())
+               return saturate(expr(ir_binop_max, x, inner_const));
          }
       }
 
-- 
2.0.5