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

[Matt Turner]

On Thu, Jul 10, 2014 at 2: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.

Nice. Thanks DX translators. :(

I'll run this on our collection of shaders too. I'll reply with some stats.

> 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%)

There's something wrong with this shader. It seems to fluctuate
between 1048 and 1052 instructions for me, even without any changes.

>
> 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];

Looks good to me.

Reviewed-by: Matt Turner <mattst88 at gmail.com>