[Mesa-dev] [PATCH 06/16] glsl: Add sin, cos and sign to get_range

[Jason Ekstrand]

On Wed, Nov 19, 2014 at 10:43 AM, Ian Romanick <idr at freedesktop.org> wrote:

> On 11/16/2014 05:51 PM, Thomas Helland wrote:
> > They are bound between -1 and 1, so report that.
> > ---
> >  src/glsl/opt_minmax.cpp | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
> > index 111d183..341006e 100644
> > --- a/src/glsl/opt_minmax.cpp
> > +++ b/src/glsl/opt_minmax.cpp
> > @@ -271,6 +271,10 @@ get_range(ir_rvalue *rval)
> >     minmax_range r0;
> >     minmax_range r1;
> >
> > +   void *mem_ctx = ralloc_parent(rval);
>
> I would constify this (as 'void *const mem_ctx') to avoid accidental,
> unintentional assignments from being added later.  We use mem_ctx a lot
> as a name, and this has happened before with nested scopes.  I know
> others on the project may not agree with me here...
>
> > +   ir_constant *low = NULL;
> > +   ir_constant *high = NULL;
> > +
> >     if(expr) {
> >        switch(expr->operation) {
> >        case ir_binop_min:
> > @@ -279,6 +283,15 @@ get_range(ir_rvalue *rval)
> >           r1 = get_range(expr->operands[1]);
> >           return combine_range(r0, r1, expr->operation == ir_binop_min);
> >
> > +      case ir_unop_sin:
> > +      case ir_unop_sin_reduced:
> > +      case ir_unop_cos:
> > +      case ir_unop_cos_reduced:
> > +      case ir_unop_sign:
> > +         high = new(mem_ctx) ir_constant(1.0f);
> > +         low = new(mem_ctx) ir_constant(-1.0f);
> > +         return minmax_range(low, high);
> > +
>
> Why not just
>
>     return minmax_range(new(mem_ctx) ir_constant(1.0f),
>                         new(mem_ctx) ir_constant(-1.0f));
>

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.


>
> >        default:
> >           break;
> >        }
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141119/ec2272fd/attachment-0001.html>