I agree, agree, and agree. We do indeed have a neg( neg() ) pass.
Writing a shader similar to the one you suggest gives clean A + B
with current master due to this, so this patch will do no good.
2014-07-15 19:05 GMT+02:00 Ian Romanick <i...@freedesktop.org>:
> I haven't really looked at the other patches yet, but the subject of
> this one caught my eye.
>
> I'm not sure this is useful. I believe we lower A-B to A+neg(B) for all
> architectures. I'm pretty sure we also already have a pass that
> converts A+neg(neg(B)) to A+B. Did you write any shaders to try to
> tickle this code? What did something like the following produce before
> and after this change?
>
> uniform vec4 a;
> uniform vec4 b;
> void main()
> {
> gl_Position = a - (-b);
> }
>
> For the neg(A)-B case, I think the existing code is better for most
> architectures. You end up with a single
>
> ADD r2, -r1, -r0
>
> instruction currently. With this change you would potentially need an
> extra move to apply the negation. I believe at least the i965 backend
> has a pass to push those negations "down" the tree... effectively
> undoing what your pass does. :)
>
> On 07/14/2014 03:22 PM, thomashellan...@gmail.com wrote:
> > From: Thomas Helland <thomashellan...@gmail.com>
> >
> > ...and neg(A) - B == neg(A + B)
> >
> > Signed-off-by: Thomas Helland <thomashellan...@gmail.com>
> > ---
> > src/glsl/opt_algebraic.cpp | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
> > index 2361d0f..fba9de6 100644
> > --- a/src/glsl/opt_algebraic.cpp
> > +++ b/src/glsl/opt_algebraic.cpp
> > @@ -454,6 +454,12 @@
> ir_algebraic_visitor::handle_expression(ir_expression *ir)
> > /* X - X == 0 */
> > if (ir->operands[0]->equals(ir->operands[1]))
> > return ir_constant::zero(ir, ir->type);
> > + /* A - neg(B) = A + B */
> > + if (op_expr[1] && op_expr[1]->operation == ir_unop_neg)
> > + return add(ir->operands[0], op_expr[1]->operands[0]);
> > + /* neg(A) - B = neg(A + B) */
> > + if (op_expr[0] && op_expr[0]->operation == ir_unop_neg)
> > + return neg(add(op_expr[0]->operands[0], ir->operands[1]));
> > break;
> >
> > case ir_binop_mul:
> >
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
