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