[Mesa-dev] [PATCH] glsl: Optimize logic operation A || (A && B)

[Brian Paul]

On 07/10/2014 03:24 PM, thomashelland90 at gmail.com wrote:
> From: Thomas Helland <thomashelland90 at gmail.com>
>
> Let's cut the needless A && B here.
> Gives some effect on a clean shader-db with
> some extra shaders from TF2 and portal.
>
> helped: shaders/tf2/2042.shader_test fs16:                23 -> 21
> (-8.70%)
> helped: shaders/tf2/2042.shader_test fs8:                 23 -> 21
> (-8.70%)
> helped: shaders/tf2/4624.shader_test fs16:                21 -> 19
> (-9.52%)
> helped: shaders/tf2/4624.shader_test fs8:                 21 -> 19
> (-9.52%)
> helped: shaders/tf2/763.shader_test fs16:                 23 -> 21
> (-8.70%)
> helped: shaders/tf2/763.shader_test fs8:                  23 -> 21
> (-8.70%)
>
> HURT:   shaders/orbital_explorer.shader_test vs:          1049 -> 1052
> (0.29%)
>
> total instructions in shared programs: 758979 -> 758970 (-0.00%)
> instructions in affected programs:     1183 -> 1174 (-0.76%)
> GAINED:                                0
> LOST:                                  0
>
> Signed-off-by: Thomas Helland <thomashelland90 at gmail.com>
> ---
>   src/glsl/opt_algebraic.cpp | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
> index ac7514a..8f3a505 100644
> --- a/src/glsl/opt_algebraic.cpp
> +++ b/src/glsl/opt_algebraic.cpp
> @@ -588,6 +588,16 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
>         } else if (ir->operands[0]->equals(ir->operands[1])) {
>            /* (a || a) == a */
>            return ir->operands[0];
> +      } else if (op_expr[0] && op_expr[0]->operation == ir_binop_logic_and &&
> +                    (op_expr[0]->operands[0]->equals(op_expr[1]) ||
> +                     op_expr[0]->operands[1]->equals(op_expr[1]))) {
> +         /* A || (A && B)  or A || (B && A) */
> +         return ir->operands[0];
> +      } else if (op_expr[1] && op_expr[1]->operation == ir_binop_logic_and &&
> +                    (op_expr[1]->operands[0]->equals(op_expr[0]) ||
> +                     op_expr[1]->operands[1]->equals(op_expr[0]))) {
> +         /* (A && B) || A  or (B && A) || A */
> +         return ir->operands[1];
>         }
>         break;
>
>

It would be helpful if the comments indicated what the expression is 
simplified to.  Ex:

/* A || (A && B) == A, or A || (B && A) == A */

But something else looks fishy here.  The comments don't seem to agree 
with the code, if I'm reading things right.

BTW, looking at these algebraic simplifications in general, where do we 
check for operands with side-effects?  For example, in the "1^x == 1" 
optimization, suppose x is a function call which modifies a global. 
Removing x would be invalid, right?

-Brian