[Mesa-dev] [PATCH] i965/fs: Fix saturate for nir_opcode_bcsel.
jason at jlekstrand.net
Tue Feb 3 12:32:47 PST 2015
On Tue, Feb 3, 2015 at 12:27 PM, Kenneth Graunke <kenneth at whitecape.org>
> 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, fs_reg(0), BRW_CONDITIONAL_NZ));
> > > inst = emit(SEL(result, op, op));
> > > 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
> > 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(...))
> 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
> 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...
We have one. It just gets lowered to a dest modifier before you see it.
That said, maybe it was getting lowered wrong here...
> 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.
Yeah, it should most of the time. However, lowering the sat in SSA has its
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the mesa-dev