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

[Jason Ekstrand]

Matt wanted shader-db numbers and here they are:

total instructions in shared programs: 5998287 -> 5974070 (-0.40%)
instructions in affected programs:     732041 -> 707824 (-3.31%)
helped:                                3135
HURT:                                  193
GAINED:                                18
LOST:                                  0


On Thu, Jan 29, 2015 at 3:41 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> On Wednesday, January 28, 2015 04:27:23 PM Jason Ekstrand wrote:
> > 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?
>
> In the brw_eu_emit.c code I worked around this by creating a brw_last_inst
> macro that always refers to the most recently emitted instruction.
>
> Still not 100% crazy about it, but it's been pretty handy.  *shrug*.
> It's an idea, anyway.
>
> I'm not too opposed to just creating fs_inst * temporaries.
>
> --Ken
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150129/a6174ea9/attachment.html>