[Mesa-dev] [PATCH] i965/fs: Fix saturate for nir_opcode_bcsel.

[Kenneth Graunke]

On Tuesday, February 03, 2015 07:10:20 AM you wrote:
> On Feb 3, 2015 2:35 AM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:
> > Caught by lit_sat.shader_test.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > index 153a1be..3c611af 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > @@ -1084,6 +1084,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
> >        emit(CMP(reg_null_d, op[0], fs_reg(0), BRW_CONDITIONAL_NZ));
> >        inst = emit(SEL(result, op[1], op[2]));
> >        inst->predicate = BRW_PREDICATE_NORMAL;
> > +      inst->saturate = instr->dest.saturate;
> >        break;
> >
> >     default:
> > --
> > 2.2.2
>
> Hrm... I thought bcsel worked on integers.  You shouldn't be able to sat it
> anyway... This seems strange.
> 
> As a side-note, this is one of the downsides to typeless that we should
> figure out how to solve.  Not 100% sure how at the moment.

For LIT's Z component, I generate different code based on whether
drivers support native integers/prefer real booleans:

   bcsel(fge(0.0f, src.x), 0.0f, pow(...))

or

   fcsel(sge(0.0f, src.x), 0.0f, pow(...))

My thinking was that bcsel uses a real boolean condition, whereas fcsel
has to do condition != 0.0f...and that the type of the sources being
selected shouldn't really matter.  But I suppose it does if we're doing
saturate.

Incidentally, making an "fsat" ALU operation would solve that ambiguity,
wouldn't require special handling all over the place, could be optimized
in nir_opt_algebraic, and would probably be better for nouveau...

Plus, I think we can probably just emit MOV.sat in the i965 backend, and 
Matt's saturation propagation pass should clean it up for us.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150203/37c62b5b/attachment.sig>