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