[Mesa-dev] [PATCH 2/3] Added support for disassembling SENDS and SENDSC.

Matt Turner mattst88 at gmail.com
Mon Feb 13 14:02:18 UTC 2017


On Mon, Feb 13, 2017 at 3:25 AM, Lonnberg, Toni <toni.lonnberg at intel.com> wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_disasm.c | 109 +++++++++++++++++++++++++++++++--
>  src/mesa/drivers/dri/i965/brw_inst.h   |  31 +++++++++-
>  2 files changed, 135 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c b/src/mesa/drivers/dri/i965/brw_disasm.c
> index 01c649c..2026312 100644
> --- a/src/mesa/drivers/dri/i965/brw_disasm.c
> +++ b/src/mesa/drivers/dri/i965/brw_disasm.c
> @@ -723,6 +723,38 @@ dest(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
>     unsigned elem_size = brw_element_size(devinfo, inst, dst);
>     int err = 0;
>
> +   if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDS ||
> +       brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDSC) {
> +      assert(devinfo->gen >= 9);

Please remove the assertions. The assembly validator will catch uses
of instructions on platforms on which they do not exist. The
disassembler should just do its job regardless and disassemble what's
there.

> +
> +      if (brw_inst_sends_dst_address_mode(devinfo, inst) == BRW_ADDRESS_DIRECT) {
> +         err |= reg(file, brw_inst_sends_dst_reg_file(devinfo, inst),
> +                    brw_inst_sends_dst_da_reg_nr(devinfo, inst));
> +
> +         if (err == -1)
> +            return 0;
> +
> +         if (brw_inst_sends_dst_da_subreg_nr(devinfo, inst))
> +            format(file, ".%"PRIu64, brw_inst_sends_dst_da_subreg_nr(devinfo, inst) /
> +                   elem_size);
> +         string(file, "<1>");
> +         err |= control(file, "dest reg encoding", reg_encoding,
> +                        brw_inst_sends_dst_reg_type(devinfo, inst), NULL);
> +      } else {
> +         string(file, "g[a0");
> +         if (brw_inst_sends_dst_ia_subreg_nr(devinfo, inst))
> +            format(file, ".%"PRIu64, brw_inst_sends_dst_ia_subreg_nr(devinfo, inst) /
> +                   elem_size);
> +         if (brw_inst_sends_dst_ia16_addr_imm(devinfo, inst))
> +            format(file, " %d", brw_inst_sends_dst_ia16_addr_imm(devinfo, inst));
> +         string(file, "]<1>");
> +         err |= control(file, "dest reg encoding", reg_encoding,
> +                        brw_inst_sends_dst_reg_type(devinfo, inst), NULL);
> +      }
> +
> +      return 0;

return err?

> +   }
> +
>     if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
>        if (brw_inst_dst_address_mode(devinfo, inst) == BRW_ADDRESS_DIRECT) {
>           err |= reg(file, brw_inst_dst_reg_file(devinfo, inst),
> @@ -1067,6 +1099,41 @@ imm(FILE *file, const struct gen_device_info *devinfo, unsigned type, brw_inst *
>  static int
>  src0(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
>  {
> +   unsigned elem_size = brw_element_size(devinfo, inst, src0);
> +   int err = 0;
> +
> +   if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDS ||
> +       brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDSC) {
> +      assert(devinfo->gen >= 9);
> +
> +      if (brw_inst_sends_src0_address_mode(devinfo, inst) == BRW_ADDRESS_DIRECT) {
> +         err |= reg(file, BRW_GENERAL_REGISTER_FILE,
> +                    brw_inst_sends_src0_da_reg_nr(devinfo, inst));
> +
> +         if (err == -1)
> +            return 0;
> +
> +         if (brw_inst_sends_src0_da_subreg_nr(devinfo, inst))
> +            format(file, ".%"PRIu64, brw_inst_sends_src0_da_subreg_nr(devinfo, inst) /
> +                   elem_size);
> +         string(file, "<1>");
> +         err |= control(file, "dest reg encoding", reg_encoding,
> +                        brw_inst_sends_dst_reg_type(devinfo, inst), NULL);
> +      } else {
> +         string(file, "g[a0");
> +         if (brw_inst_sends_src0_ia_subreg_nr(devinfo, inst))
> +            format(file, ".%"PRIu64, brw_inst_sends_src0_ia_subreg_nr(devinfo, inst) /
> +                   elem_size);
> +         if (brw_inst_sends_src0_ia16_addr_imm(devinfo, inst))
> +            format(file, " %d", brw_inst_sends_src0_ia16_addr_imm(devinfo, inst));
> +         string(file, "]<1>");
> +         err |= control(file, "dest reg encoding", reg_encoding,
> +                        brw_inst_sends_dst_reg_type(devinfo, inst), NULL);
> +      }
> +
> +      return 0;

return err?

> +   }
> +
>     if (brw_inst_src0_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE) {
>        return imm(file, devinfo, brw_inst_src0_reg_type(devinfo, inst), inst);
>     } else if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
> @@ -1123,6 +1190,22 @@ src0(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
>  static int
>  src1(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
>  {
> +   int err = 0;
> +
> +   if (brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDS ||
> +       brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDSC) {
> +      assert(devinfo->gen >= 9);
> +
> +      err |= reg(file, brw_inst_sends_src1_reg_file(devinfo, inst),
> +                 brw_inst_sends_src1_da_reg_nr(devinfo, inst));
> +
> +      string(file, "<1>");
> +      err |= control(file, "dest reg encoding", reg_encoding,
> +                     brw_inst_sends_dst_reg_type(devinfo, inst), NULL);
> +
> +      return 0;

return err, right?

> +   }
> +
>     if (brw_inst_src1_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE) {
>        return imm(file, devinfo, brw_inst_src1_reg_type(devinfo, inst), inst);
>     } else if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
> @@ -1261,7 +1344,8 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>        string(file, " ");
>        err |= control(file, "function", math_function,
>                       brw_inst_math_function(devinfo, inst), NULL);
> -   } else if (opcode != BRW_OPCODE_SEND && opcode != BRW_OPCODE_SENDC) {
> +   } else if (opcode != BRW_OPCODE_SEND && opcode != BRW_OPCODE_SENDC &&
> +              opcode != BRW_OPCODE_SENDS && opcode != BRW_OPCODE_SENDSC) {
>        err |= control(file, "conditional modifier", conditional_modifier,
>                       brw_inst_cond_modifier(devinfo, inst), NULL);
>
> @@ -1390,6 +1474,16 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>
>           pad(file, 80);
>           err |= src1(file, devinfo, inst);
> +      } else if (opcode == BRW_OPCODE_SENDS || opcode == BRW_OPCODE_SENDSC) {
> +         pad(file, 64);
> +         format(file, "0x%"PRIx32, brw_inst_sends_exdesc(devinfo, inst));
> +
> +         pad(file, 80);
> +         if (brw_inst_sends_selreg32desc(devinfo, inst)) {
> +            format(file, "a0.0<0;1,0>:ud");
> +         } else {
> +            format(file, "0x%"PRIx32, brw_inst_imm_ud(devinfo, inst));
> +         }
>        }
>     }
>
> @@ -1434,7 +1528,8 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>           err |= control(file, "acc write control", accwr,
>                          brw_inst_acc_wr_control(devinfo, inst), &space);
>        }
> -      if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC)
> +      if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC ||
> +          opcode == BRW_OPCODE_SENDS || opcode == BRW_OPCODE_SENDSC)
>           err |= control(file, "end of thread", end_of_thread,
>                          brw_inst_eot(devinfo, inst), &space);
>        if (space)
> @@ -1443,7 +1538,8 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>     }
>     string(file, ";");
>
> -   if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) {
> +   if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC ||
> +       opcode == BRW_OPCODE_SENDS || opcode == BRW_OPCODE_SENDSC) {
>        enum brw_message_target sfid = brw_inst_sfid(devinfo, inst);
>
>        space = 0;
> @@ -1453,7 +1549,8 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>                       sfid, &space);
>
>
> -      if ((opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) && (brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE)) {
> +      if (((opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) && (brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE)) ||
> +         ((opcode == BRW_OPCODE_SENDS || opcode == BRW_OPCODE_SENDSC) && brw_inst_sends_selreg32desc(devinfo, inst))) {
>           format(file, " indirect");
>        } else {
>           switch (sfid) {
> @@ -1675,6 +1772,10 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>              string(file, " ");
>           format(file, "mlen %"PRIu64, brw_inst_mlen(devinfo, inst));
>           format(file, " rlen %"PRIu64, brw_inst_rlen(devinfo, inst));
> +
> +         if (opcode == BRW_OPCODE_SENDS || opcode == BRW_OPCODE_SENDSC) {
> +            format(file, " exdesc len %"PRIu64, brw_inst_sends_exdesc_len(devinfo, inst));
> +         }
>        }
>     } else {
>        if (brw_inst_src0_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE) {
> diff --git a/src/mesa/drivers/dri/i965/brw_inst.h b/src/mesa/drivers/dri/i965/brw_inst.h
> index bcb6786..33b87d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_inst.h
> +++ b/src/mesa/drivers/dri/i965/brw_inst.h
> @@ -244,6 +244,33 @@ F(3src_opcode,           6,  0)
>  /** @} */
>
>  /**
> + * SENDS and SENDSC instructions:
> + *  @{
> + */

There are a couple of fields in here that mirror the regular 1-2 src
instructions. If adding them makes the code they're used in clearer,
that's okay, but I'm finding them to make it difficult to see what's
actually different.

>From the docs, it looks like nearly all the changes are summarized by:
new ExDesc field; new SelReg32Desc field; new SelReg32ExDesc field;
and src1 fields are moved; something different about src0/dst AddrImm.

The ones marked below are the same as in regular instructions. I don't
know whether we should keep them. What are your thoughts?

> +F(sends_src0_address_mode,   79,  79)

This one

> +F(sends_selreg32desc,        77,  77)
> +F(sends_src0_ia_subreg_nr,   76,  73)
> +F(sends_src0_da_reg_nr,      76,  69)

These two

> +F(sends_src0_da_subreg_nr,   68,  68)
> +F(sends_exdesc_len,          67,  64)
> +F(sends_dst_address_mode,    63,  63)
> +F(sends_dst_ia_subreg_nr,    60,  57)
> +F(sends_dst_da_reg_nr,       60,  53)
> +F(sends_dst_da_subreg_nr,    52,  52)

These four.

> +F(sends_src1_da_reg_nr,      51,  44)
> +F(sends_dst_reg_type,        40,  37)

This one

> +F(sends_src1_reg_file,       36,  36)
> +F(sends_dst_reg_file,        35,  35)

This one

I don't see a function for SelReg32ExDesc. Don't we need that?


More information about the mesa-dev mailing list