<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 3, 2015 at 12:27 PM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</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 Tuesday, February 03, 2015 07:10:20 AM you wrote:<br>
> On Feb 3, 2015 2:35 AM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
> > Caught by lit_sat.shader_test.<br>
> ><br>
> > Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
> > ---<br>
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 1 +<br>
> >  1 file changed, 1 insertion(+)<br>
> ><br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> > index 153a1be..3c611af 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> > @@ -1084,6 +1084,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)<br>
> >        emit(CMP(reg_null_d, op[0], fs_reg(0), BRW_CONDITIONAL_NZ));<br>
> >        inst = emit(SEL(result, op[1], op[2]));<br>
> >        inst->predicate = BRW_PREDICATE_NORMAL;<br>
> > +      inst->saturate = instr->dest.saturate;<br>
> >        break;<br>
> ><br>
> >     default:<br>
> > --<br>
> > 2.2.2<br>
><br>
> Hrm... I thought bcsel worked on integers.  You shouldn't be able to sat it<br>
> anyway... This seems strange.<br>
><br>
> As a side-note, this is one of the downsides to typeless that we should<br>
> figure out how to solve.  Not 100% sure how at the moment.<br>
<br>
</div></div>For LIT's Z component, I generate different code based on whether<br>
drivers support native integers/prefer real booleans:<br>
<br>
   bcsel(fge(0.0f, src.x), 0.0f, pow(...))<br>
<br>
or<br>
<br>
   fcsel(sge(0.0f, src.x), 0.0f, pow(...))<br>
<br>
My thinking was that bcsel uses a real boolean condition, whereas fcsel<br>
has to do condition != 0.0f...and that the type of the sources being<br>
selected shouldn't really matter.  But I suppose it does if we're doing<br>
saturate.<br></blockquote><div><br></div><div>Exactly.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Incidentally, making an "fsat" ALU operation would solve that ambiguity,<br>
wouldn't require special handling all over the place, could be optimized<br>
in nir_opt_algebraic, and would probably be better for nouveau...<br></blockquote><div><br></div><div>We have one.  It just gets lowered to a dest modifier before you see it.  That said, maybe it was getting lowered wrong here...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Plus, I think we can probably just emit MOV.sat in the i965 backend, and<br>
Matt's saturation propagation pass should clean it up for us.<br>
</blockquote></div><br></div><div class="gmail_extra">Yeah, it should most of the time.  However, lowering the sat in SSA has its advantages.<br></div></div>