[Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages

[Pohjolainen, Topi]

On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote:
> Topi Pohjolainen <topi.pohjolainen at intel.com> writes:
> 
> > The original patch from Curro was based on something that is not
> > present in the master yet. This patch tries to mimick the logic on
> > top master.
> > I wanted to see if could separate the building of the descriptor
> > instruction from building of the send instruction. This logic now
> > allows the caller to construct any kind of sequence of instructions
> > filling in the descriptor before giving it to the send instruction
> > builder.
> >
> > This is only compile tested. Curro, how would you feel about this
> > sort of approach? I apologise for muddying the waters again but I
> > wasn't entirely comfortable with the logic and wanted to try to
> > something else.
> >
> > I believe patch number five should go nicely on top of this as
> > the descriptor instruction could be followed by (or preceeeded by)
> > any additional instructions modifying the descriptor register
> > before the actual send instruction.
> >
> 
> Topi, the goal I had in mind with PATCH 01 was to refactor a commonly
> recurring pattern.  In terms of the functions defined in this patch my
> example from yesterday [1] would now look like:
> 
> |   if (index.file == BRW_IMMEDIATE_VALUE) {
> |
> |      uint32_t surf_index = index.dw1.ud;
> |
> |      brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND);
> |      brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW));
> |      brw_set_src0(p, send, offset);
> |      brw_set_sampler_message(p, send,
> |                              surf_index,
> |                              0, /* LD message ignores sampler unit */
> |                              GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
> |                              rlen,
> |                              mlen,
> |                              false, /* no header */
> |                              simd_mode,
> |                              0);
> |
> |      brw_mark_surface_used(prog_data, surf_index);
> |
> |   } else {
> |
> |      struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD));
> |
> |      brw_push_insn_state(p);
> |      brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> |      brw_set_default_access_mode(p, BRW_ALIGN_1);
> |
> |      /* a0.0 = surf_index & 0xff */
> |      brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND);
> |      brw_inst_set_exec_size(p->brw, insn_and, BRW_EXECUTE_1);
> |      brw_set_dest(p, insn_and, addr);
> |      brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD)));
> |      brw_set_src1(p, insn_and, brw_imm_ud(0x0ff));
> |
> |
> |      /* a0.0 |= <descriptor> */
> |      brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr);
> |      brw_set_sampler_message(p, descr_inst,
> |                              0 /* surface */,
> |                              0 /* sampler */,
> |                              GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
> |                              rlen /* rlen */,
> |                              mlen /* mlen */,
> |                              false /* header */,
> |                              simd_mode,
> |                              0);
> |
> |      /* dst = send(offset, a0.0) */
> |      brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr);
> |
> |      brw_pop_insn_state(p);
> |
> |      /* visitor knows more than we do about the surface limit required,
> |       * so has already done marking.
> |       */
> |   }

Which I think could also be written as follows. Or am I missing something
again?

static brw_inst *
brw_build_surface_index_descr(struct brw_compile *p,
                              struct brw_reg dst, index)
{
   brw_set_default_mask_control(p, BRW_MASK_DISABLE);
   brw_set_default_access_mode(p, BRW_ALIGN_1);

   /* a0.0 = surf_index & 0xff */
   brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND);
   brw_inst_set_exec_size(p->brw, insn_and, BRW_EXECUTE_1);
   brw_set_dest(p, insn_and, addr);
   brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD)));
   brw_set_src1(p, insn_and, brw_imm_ud(0x0ff));

   /* a0.0 |= <descriptor> */
   brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr);
}

...
   brw_inst *descr_inst;
   if (index.file == BRW_IMMEDIATE_VALUE) {
      descr = brw_next_insn(p, BRW_OPCODE_SEND);
      brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW));
      brw_set_src0(p, send, offset);

      brw_mark_surface_used(prog_data, surf_index);
   } else {
      struct brw_reg addr = vec1(retype(brw_address_reg(0),
                                 BRW_REGISTER_TYPE_UD));
      brw_push_insn_state(p);

      brw_build_surface_index_descr(p, addr, index);
      /* dst = send(offset, a0.0) */
      descr_inst = brw_send_indirect_message(p, BRW_SFID_SAMPLER,
                                             dst, offset, addr);
      brw_pop_insn_state(p);
   }

   uint32_t surf_index = index.file == BRW_IMMEDIATE_VALUE ? index.dw1.ud : 0;
   brw_set_sampler_message(p, descr_inst,
                           surf_index,
                           0, /* LD message ignores sampler unit */
                           GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
                           rlen,
                           mlen,
                           false, /* no header */
                           simd_mode,
                           0);
...