On 07/10/2014 04:12 PM, Brian Paul wrote: > On 07/10/2014 03:24 PM, thomashellan...@gmail.com wrote: >> From: Thomas Helland <thomashellan...@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 <thomashellan...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev