On Mon, Feb 13, 2017 at 3:25 AM, Lonnberg, Toni <[email protected]> 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?
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev