<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 19, 2014 at 10:43 AM, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 11/16/2014 05:51 PM, Thomas Helland wrote:<br>
> They are bound between -1 and 1, so report that.<br>
> ---<br>
>  src/glsl/opt_minmax.cpp | 13 +++++++++++++<br>
>  1 file changed, 13 insertions(+)<br>
><br>
> diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp<br>
> index 111d183..341006e 100644<br>
> --- a/src/glsl/opt_minmax.cpp<br>
> +++ b/src/glsl/opt_minmax.cpp<br>
> @@ -271,6 +271,10 @@ get_range(ir_rvalue *rval)<br>
>     minmax_range r0;<br>
>     minmax_range r1;<br>
><br>
> +   void *mem_ctx = ralloc_parent(rval);<br>
<br>
</span>I would constify this (as 'void *const mem_ctx') to avoid accidental,<br>
unintentional assignments from being added later.  We use mem_ctx a lot<br>
as a name, and this has happened before with nested scopes.  I know<br>
others on the project may not agree with me here...<br>
<span class=""><br>
> +   ir_constant *low = NULL;<br>
> +   ir_constant *high = NULL;<br>
> +<br>
>     if(expr) {<br>
>        switch(expr->operation) {<br>
>        case ir_binop_min:<br>
> @@ -279,6 +283,15 @@ get_range(ir_rvalue *rval)<br>
>           r1 = get_range(expr->operands[1]);<br>
>           return combine_range(r0, r1, expr->operation == ir_binop_min);<br>
><br>
> +      case ir_unop_sin:<br>
> +      case ir_unop_sin_reduced:<br>
> +      case ir_unop_cos:<br>
> +      case ir_unop_cos_reduced:<br>
> +      case ir_unop_sign:<br>
> +         high = new(mem_ctx) ir_constant(1.0f);<br>
> +         low = new(mem_ctx) ir_constant(-1.0f);<br>
> +         return minmax_range(low, high);<br>
> +<br>
<br>
</span>Why not just<br>
<br>
    return minmax_range(new(mem_ctx) ir_constant(1.0f),<br>
                        new(mem_ctx) ir_constant(-1.0f));<br></blockquote><div><br></div><div>We can also do a lot better here if we have a small angle.  If the angle is in the range [-pi, pi], then abs(sin(x)) <= abs(x).  I have no idea if we're ever going to see a shader in the wild where we would be able to take advantage of this, but it's something to think about.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
>        default:<br>
>           break;<br>
<div class="HOEnZb"><div class="h5">>        }<br>
><br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>