[Mesa-dev] [PATCH v4 2/3] i965/fs: Use NIR's scalarizing abilities and stop handling vectors

[Jason Ekstrand]

On Wed, Jan 28, 2015 at 4:02 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On Tue, Jan 27, 2015 at 5:31 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > index de0d780..217fe09 100644
> > @@ -689,9 +689,9 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
> >
> >        struct brw_reg acc = retype(brw_acc_reg(dispatch_width),
> result.type);
> >
> > -      emit_percomp(MUL(acc, op[0], op[1]), instr->dest.write_mask);
> > -      emit_percomp(MACH(reg_null_d, op[0], op[1]),
> instr->dest.write_mask);
> > -      emit_percomp(MOV(result, fs_reg(acc)), instr->dest.write_mask);
>
> Bah. How the hell did the old code pass piglit?
>

I think channel expressions saved it.


>
> > +      emit(MUL(acc, op[0], op[1]));
> > +      emit(MACH(reg_null_d, op[0], op[1]));
> > +      emit(MOV(result, fs_reg(acc)));
> >        break;
> >     }
> >
> > @@ -773,72 +767,38 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
> >     case nir_op_ball_fequal3:
> >     case nir_op_ball_iequal3:
> >     case nir_op_ball_fequal4:
> > -   case nir_op_ball_iequal4: {
> > -      unsigned num_components = nir_op_infos[instr->op].input_sizes[0];
> > -      fs_reg temp = vgrf(num_components);
> > -      emit_percomp(CMP(temp, op[0], op[1], BRW_CONDITIONAL_Z),
> > -                   (1 << num_components) - 1);
> > -      emit_reduction(BRW_OPCODE_AND, result, temp, num_components);
> > -      break;
> > -   }
> > -
> > +   case nir_op_ball_iequal4:
>
> We can save it for later, but it might be interesting to let the fs
> backend get the vector comparisons directly, if that's possible.
>
> See https://bugs.freedesktop.org/show_bug.cgi?id=77456
>

We can.  We just have to add a flag to turn that off in the
nir_lower_alu_to_scalar pass.


>
> > @@ -867,83 +827,67 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
> >        unreachable("not reached: should be handled by ldexp_to_arith()");
> >
> >     case nir_op_fsqrt:
> > -      emit_math_percomp(SHADER_OPCODE_SQRT, result, op[0],
> > -                        instr->dest.write_mask, instr->dest.saturate);
> > +      emit_math(SHADER_OPCODE_SQRT, result, op[0])
> > +               ->saturate = instr->dest.saturate;
>
> I don't really like the ->saturate = ... as a single statement. I know
> I'm guilty of using it in a unit test recently.
>

Yeah, I'm not sure what to do there.  I really don't like having to assign
it to a fs_inst variable and then do the saturate.  I guess we could.  Or
we could add something to emit_math.  Thoughts?


> I guess if we remove saturate as a destination modifier from NIR we'll
> get rid of most instances of this pattern, leaving just ->predicate.
> Are we planning to remove src/dst modifiers from NIR?
>

Not until we get SSA in the backend.  It's so much easier to apply them in
SSA.  If we'd like to drop one of them we can always add some flags to the
lowering pass that adds them back in.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150128/27c7a042/attachment.html>