[Mesa-dev] [PATCH 8/9] i965/vec4: Handle saturated constants in opt_vector_float

[Jason Ekstrand]

On Thu, Mar 17, 2016 at 11:55 AM, Matt Turner <mattst88 at gmail.com> wrote:

> On Thu, Mar 17, 2016 at 11:52 AM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Thu, Mar 17, 2016 at 11:18 AM, Matt Turner <mattst88 at gmail.com>
> wrote:
> >>
> >> On Thu, Mar 17, 2016 at 10:21 AM, Jason Ekstrand <jason at jlekstrand.net>
> >> wrote:
> >> > ---
> >> >  src/mesa/drivers/dri/i965/brw_vec4.cpp | 8 +++++++-
> >> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> > b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> > index 155a550..02a00b3 100644
> >> > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> >> > @@ -384,7 +384,13 @@ vec4_visitor::opt_vector_float()
> >> >           continue;
> >> >        }
> >> >
> >> > -      int vf = brw_float_to_vf(inst->src[0].f);
> >> > +      float f = inst->src[0].f;
> >> > +      if (inst->saturate) {
> >> > +         assert(inst->dst.type == BRW_REGISTER_TYPE_F);
> >> > +         f = CLAMP(f, 0.0f, 1.0f);
> >> > +      }
> >> > +
> >> > +      int vf = brw_float_to_vf(f);
> >> >        if (vf == -1)
> >> >           continue;
> >>
> >> Presumably the previous patch is to allow this to happen without
> >> thinking about types.
> >>
> >> This does look like a legitimate bug fix, but what does this fix or
> >> enable?
> >
> >
> > Patch 9 removes the mov.sat optimization from opt_algebraic which was
> fixing
> > up saturated constants before opt_vector_float ever saw them.  When I
> > removed it, I got a bunch of piglit fails from things that were writing
> > out-of-bounds constant values to auto-clampped outputs.  The problem was
> > that opt_vector_float was just throwing the .sat away.
>
> Oh, so this isn't even a legitimate bug fix unless you remove the code
> this relies on...
>
> Just keep the saturating code in opt_algebraic. Why would we remove it...?
>

Should we be trusting in opt_algebraic for correctness?  That doesn't seem
right.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160317/3eb37e4d/attachment.html>