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

[Ian Romanick]

On 07/10/2014 04:12 PM, Brian Paul wrote:
> 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.

It looks like the comments are backwards with the code.  That is, /* (A
&& B) || A  or (B && A) || A */ shouldbe with the op_expr[0] case
instead of the op_expr[1] case.

> 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
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>