[Mesa-dev] [PATCH 1/3] Added label support for shader disassembly. SEND and SENDC formatting now also includes the SFID information.
Matt Turner
mattst88 at gmail.com
Mon Feb 13 13:31:00 UTC 2017
Patches' titles should have a prefix. These should likely be "i965: "
Also, I don't know what you mean by a label. The target of an
unstructured jump? The commit message should explain and provide some
background information.
On Mon, Feb 13, 2017 at 3:25 AM, Lonnberg, Toni <toni.lonnberg at intel.com> wrote:
> ---
> src/intel/tools/aubinator.c | 48 ++----
> src/intel/tools/disasm.c | 63 ++++++--
> src/mesa/drivers/dri/i965/brw_context.h | 4 +-
> src/mesa/drivers/dri/i965/brw_disasm.c | 193 ++++++++++++++++---------
> src/mesa/drivers/dri/i965/brw_eu.c | 119 ++++++++++++---
> src/mesa/drivers/dri/i965/brw_eu.h | 11 ++
> src/mesa/drivers/dri/i965/brw_eu_compact.c | 4 +-
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 8 +-
> src/mesa/drivers/dri/i965/intel_debug.c | 1 +
> src/mesa/drivers/dri/i965/intel_debug.h | 1 +
> src/mesa/drivers/dri/i965/test_eu_compact.c | 2 +-
> 11 files changed, 309 insertions(+), 145 deletions(-)
>
> diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
> index e2bec8f..ed649fa 100644
> --- a/src/intel/tools/aubinator.c
> +++ b/src/intel/tools/aubinator.c
> @@ -42,6 +42,7 @@
> #include "decoder.h"
> #include "intel_aub.h"
> #include "gen_disasm.h"
> +#include "brw_eu.h"
>
> /* Below is the only command missing from intel_aub.h in libdrm
> * So, reuse intel_aub.h from libdrm and #define the
> @@ -324,6 +325,7 @@ handle_media_interface_descriptor_load(struct gen_spec *spec, uint32_t *p)
>
> descriptor_structure =
> gen_spec_find_struct(spec, "INTERFACE_DESCRIPTOR_DATA");
> +
Unrelated change.
> if (descriptor_structure == NULL) {
> printf("did not find INTERFACE_DESCRIPTOR_DATA info\n");
> return;
> @@ -671,43 +673,10 @@ handle_load_register_imm(struct gen_spec *spec, uint32_t *p)
>
> #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
>
> -#define STATE_BASE_ADDRESS 0x61010000
> -
> -#define MEDIA_INTERFACE_DESCRIPTOR_LOAD 0x70020000
> -
> -#define _3DSTATE_INDEX_BUFFER 0x780a0000
> -#define _3DSTATE_VERTEX_BUFFERS 0x78080000
> -
> -#define _3DSTATE_VS 0x78100000
> -#define _3DSTATE_GS 0x78110000
> -#define _3DSTATE_HS 0x781b0000
> -#define _3DSTATE_DS 0x781d0000
> -
> -#define _3DSTATE_CONSTANT_VS 0x78150000
> -#define _3DSTATE_CONSTANT_GS 0x78160000
> -#define _3DSTATE_CONSTANT_PS 0x78170000
> -#define _3DSTATE_CONSTANT_HS 0x78190000
> -#define _3DSTATE_CONSTANT_DS 0x781A0000
> -
> -#define _3DSTATE_PS 0x78200000
> -
> -#define _3DSTATE_BINDING_TABLE_POINTERS_VS 0x78260000
> -#define _3DSTATE_BINDING_TABLE_POINTERS_HS 0x78270000
> -#define _3DSTATE_BINDING_TABLE_POINTERS_DS 0x78280000
> -#define _3DSTATE_BINDING_TABLE_POINTERS_GS 0x78290000
> -#define _3DSTATE_BINDING_TABLE_POINTERS_PS 0x782a0000
> -
> -#define _3DSTATE_SAMPLER_STATE_POINTERS_VS 0x782b0000
> -#define _3DSTATE_SAMPLER_STATE_POINTERS_GS 0x782e0000
> -#define _3DSTATE_SAMPLER_STATE_POINTERS_PS 0x782f0000
> -
> -#define _3DSTATE_VIEWPORT_STATE_POINTERS_CC 0x78230000
> -#define _3DSTATE_VIEWPORT_STATE_POINTERS_SF_CLIP 0x78210000
> -#define _3DSTATE_BLEND_STATE_POINTERS 0x78240000
> -#define _3DSTATE_CC_STATE_POINTERS 0x780e0000
> -#define _3DSTATE_SCISSOR_STATE_POINTERS 0x780f0000
> -
> -#define _MI_LOAD_REGISTER_IMM 0x11000000
> +#define STATE_BASE_ADDRESS 0x6101
> +#define _3DSTATE_INDEX_BUFFER 0x780a
> +#define _3DSTATE_VIEWPORT_STATE_POINTERS_SF_CLIP 0x7821
> +#define _MI_LOAD_REGISTER_IMM 0x1100
What is going on here? This all seems to be unrelated to the patch?
>
> struct custom_handler {
> uint32_t opcode;
> @@ -808,8 +777,7 @@ parse_commands(struct gen_spec *spec, uint32_t *cmds, int size, int engine)
> }
>
> for (i = 0; i < ARRAY_LENGTH(custom_handlers); i++) {
> - if (gen_group_get_opcode(inst) ==
> - custom_handlers[i].opcode)
> + if (gen_group_get_opcode(inst) == (custom_handlers[i].opcode << 16))
> custom_handlers[i].handle(spec, p);
> }
> }
> @@ -1240,6 +1208,8 @@ int main(int argc, char *argv[])
> { NULL, 0, NULL, 0 }
> };
>
> + brw_process_intel_debug_variable();
> +
> i = 0;
> while ((c = getopt_long(argc, argv, "", aubinator_opts, &i)) != -1) {
> switch (c) {
> diff --git a/src/intel/tools/disasm.c b/src/intel/tools/disasm.c
> index 4ac7b90..a8c3786 100644
> --- a/src/intel/tools/disasm.c
> +++ b/src/intel/tools/disasm.c
> @@ -49,40 +49,81 @@ gen_disasm_disassemble(struct gen_disasm *disasm, void *assembly,
> int start, FILE *out)
> {
> struct gen_device_info *devinfo = &disasm->devinfo;
> - bool dump_hex = false;
> + bool dump_hex = (INTEL_DEBUG & DEBUG_HEX) != 0;
> int offset = start;
>
> + void *mem_ctx = ralloc_context(NULL);
> + struct brw_label *root = NULL;
> +
> + if ((INTEL_DEBUG & DEBUG_DISASM) != 0) {
> + while (true) {
> + brw_inst *insn = assembly + offset;
> + brw_inst uncompacted;
> +
> + bool compacted = brw_inst_cmpt_control(devinfo, insn);
> +
> + if (compacted) {
> + brw_compact_inst *compacted = (void *)insn;
> + brw_uncompact_instruction(devinfo, &uncompacted, compacted);
> + insn = &uncompacted;
> + }
> +
> + if (brw_has_uip(devinfo, brw_inst_opcode(devinfo, insn)))
> + brw_create_label(&root, offset + brw_inst_uip(devinfo, insn), mem_ctx);
> +
> + if (brw_has_jip(devinfo, brw_inst_opcode(devinfo, insn)))
> + brw_create_label(&root, offset + brw_inst_jip(devinfo, insn), mem_ctx);
> +
> + offset += compacted ? 8 : 16;
> +
> + uint32_t opcode = brw_inst_opcode(devinfo, insn);
> + if (opcode == 0 || (is_send(opcode) && brw_inst_eot(devinfo, insn))) {
> + break;
> + }
Indentation is wrong here.
> + }
> + }
> +
> /* This loop exits when send-with-EOT or when opcode is 0 */
> + offset = start;
> while (true) {
> brw_inst *insn = assembly + offset;
> brw_inst uncompacted;
> +
> + if (root != NULL) {
> + const struct brw_label *label = brw_find_label(root, offset);
> + if (label != NULL) {
> + fprintf(out, "\n_label%d:\n", label->number);
> + }
> + }
> +
> bool compacted = brw_inst_cmpt_control(devinfo, insn);
> +
> if (0)
> fprintf(out, "0x%08x: ", offset);
>
> if (compacted) {
> brw_compact_inst *compacted = (void *)insn;
> +
> if (dump_hex) {
> fprintf(out, "0x%08x 0x%08x ",
> - ((uint32_t *)insn)[1],
> - ((uint32_t *)insn)[0]);
> + ((uint32_t *)insn)[1],
> + ((uint32_t *)insn)[0]);
> }
>
> brw_uncompact_instruction(devinfo, &uncompacted, compacted);
> insn = &uncompacted;
> - offset += 8;
> } else {
> if (dump_hex) {
> fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ",
> - ((uint32_t *)insn)[3],
> - ((uint32_t *)insn)[2],
> - ((uint32_t *)insn)[1],
> - ((uint32_t *)insn)[0]);
> + ((uint32_t *)insn)[3],
> + ((uint32_t *)insn)[2],
> + ((uint32_t *)insn)[1],
> + ((uint32_t *)insn)[0]);
Looks like just more stray changes?
> }
> - offset += 16;
> }
>
> - brw_disassemble_inst(out, devinfo, insn, compacted);
> + brw_disassemble_inst(out, devinfo, insn, compacted, offset, root);
> + offset += compacted ? 8 : 16;
>
> /* Simplistic, but efficient way to terminate disasm */
> uint32_t opcode = brw_inst_opcode(devinfo, insn);
> @@ -90,6 +131,8 @@ gen_disasm_disassemble(struct gen_disasm *disasm, void *assembly,
> break;
> }
> }
> +
> + ralloc_free(mem_ctx);
> }
>
> struct gen_disasm *
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 01e651b..850d882 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -154,6 +154,7 @@ struct brw_wm_prog_key;
> struct brw_wm_prog_data;
> struct brw_cs_prog_key;
> struct brw_cs_prog_data;
> +struct brw_label;
>
> enum brw_pipeline {
> BRW_RENDER_PIPELINE,
> @@ -1366,7 +1367,8 @@ void brw_vec4_alloc_reg_set(struct brw_compiler *compiler);
>
> /* brw_disasm.c */
> int brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
> - struct brw_inst *inst, bool is_compacted);
> + struct brw_inst *inst, bool is_compacted,
> + int offset, struct brw_label *label_root);
>
> /* brw_vs.c */
> gl_clip_plane *brw_select_clip_planes(struct gl_context *ctx);
> diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c b/src/mesa/drivers/dri/i965/brw_disasm.c
> index cd553b3..01c649c 100644
> --- a/src/mesa/drivers/dri/i965/brw_disasm.c
> +++ b/src/mesa/drivers/dri/i965/brw_disasm.c
> @@ -30,8 +30,8 @@
> #include "brw_inst.h"
> #include "brw_eu.h"
>
> -static bool
> -has_jip(const struct gen_device_info *devinfo, enum opcode opcode)
> +bool
> +brw_has_jip(const struct gen_device_info *devinfo, enum opcode opcode)
> {
> if (devinfo->gen < 6)
> return false;
> @@ -45,8 +45,8 @@ has_jip(const struct gen_device_info *devinfo, enum opcode opcode)
> opcode == BRW_OPCODE_HALT;
> }
>
> -static bool
> -has_uip(const struct gen_device_info *devinfo, enum opcode opcode)
> +bool
> +brw_has_uip(const struct gen_device_info *devinfo, enum opcode opcode)
> {
> if (devinfo->gen < 6)
> return false;
> @@ -1218,13 +1218,14 @@ brw_disassemble_imm(const struct gen_device_info *devinfo,
> brw_inst inst;
> inst.data[0] = (((uint64_t) dw1) << 32) | ((uint64_t) dw0);
> inst.data[1] = (((uint64_t) dw3) << 32) | ((uint64_t) dw2);
> - return brw_disassemble_inst(stderr, devinfo, &inst, false);
> + return brw_disassemble_inst(stderr, devinfo, &inst, false, 0, NULL);
> }
> #endif
>
> int
> brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
> - brw_inst *inst, bool is_compacted)
> + brw_inst *inst, bool is_compacted,
> + int offset, struct brw_label *label_root)
> {
> int err = 0;
> int space = 0;
> @@ -1289,18 +1290,53 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
> if (opcode == BRW_OPCODE_SEND && devinfo->gen < 6)
> format(file, " %"PRIu64, brw_inst_base_mrf(devinfo, inst));
>
> - if (has_uip(devinfo, opcode)) {
> + if (brw_has_uip(devinfo, opcode)) {
> /* Instructions that have UIP also have JIP. */
> - pad(file, 16);
> - format(file, "JIP: %d", brw_inst_jip(devinfo, inst));
> - pad(file, 32);
> - format(file, "UIP: %d", brw_inst_uip(devinfo, inst));
> - } else if (has_jip(devinfo, opcode)) {
> - pad(file, 16);
> - if (devinfo->gen >= 7) {
> + if (label_root != NULL) {
> + const struct brw_label *branch = NULL;
> +
> + pad(file, 16);
> + branch = brw_find_label(label_root, offset + brw_inst_jip(devinfo, inst));
> + if (branch != NULL)
> + format(file, "_label%d", branch->number);
> + else
> + format(file, "JIP: %d", brw_inst_jip(devinfo, inst));
> +
> + pad(file, 32);
> + branch = brw_find_label(label_root, offset + brw_inst_uip(devinfo, inst));
> + if (branch != NULL)
> + format(file, "_label%d", branch->number);
> + else
> + format(file, "UIP: %d", brw_inst_uip(devinfo, inst));
> + } else {
> + pad(file, 16);
> format(file, "JIP: %d", brw_inst_jip(devinfo, inst));
> + pad(file, 32);
> + format(file, "UIP: %d", brw_inst_uip(devinfo, inst));
> + }
> + } else if (brw_has_jip(devinfo, opcode)) {
> + if (label_root != NULL) {
> + const struct brw_label *branch = NULL;
> + int jip;
> +
> + if (devinfo->gen >= 7)
> + jip = brw_inst_jip(devinfo, inst);
> + else
> + jip = brw_inst_gen6_jump_count(devinfo, inst);
> +
> + pad(file, 16);
> + branch = brw_find_label(label_root, offset + jip);
> + if (branch != NULL)
> + format(file, "_label%d", branch->number);
> + else
> + format(file, "JIP: %d", jip);
Please use braces for nested control flow.
> } else {
> - format(file, "JIP: %d", brw_inst_gen6_jump_count(devinfo, inst));
> + pad(file, 16);
> + if (devinfo->gen >= 7) {
> + format(file, "JIP: %d", brw_inst_jip(devinfo, inst));
> + } else {
> + format(file, "JIP: %d", brw_inst_gen6_jump_count(devinfo, inst));
> + }
> }
> } else if (devinfo->gen < 6 && (opcode == BRW_OPCODE_BREAK ||
> opcode == BRW_OPCODE_CONTINUE ||
> @@ -1347,27 +1383,77 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
> pad(file, 48);
> err |= src1(file, devinfo, inst);
> }
> - }
>
> - if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) {
> - enum brw_message_target sfid = brw_inst_sfid(devinfo, inst);
> + if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) {
> + pad(file, 64);
> + format(file, "0x%"PRIx64, brw_inst_sfid(devinfo, inst));
>
> - if (brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE) {
> - /* show the indirect descriptor source */
> - pad(file, 48);
> + pad(file, 80);
> err |= src1(file, devinfo, inst);
> }
> + }
> +
> + pad(file, 64);
> + if (opcode != BRW_OPCODE_NOP && opcode != BRW_OPCODE_NENOP) {
> + string(file, "{");
> + space = 1;
> + err |= control(file, "access mode", access_mode,
> + brw_inst_access_mode(devinfo, inst), &space);
> + if (devinfo->gen >= 6) {
> + err |= control(file, "write enable control", wectrl,
> + brw_inst_mask_control(devinfo, inst), &space);
> + } else {
> + err |= control(file, "mask control", mask_ctrl,
> + brw_inst_mask_control(devinfo, inst), &space);
> + }
> + err |= control(file, "dependency control", dep_ctrl,
> + ((brw_inst_no_dd_check(devinfo, inst) << 1) |
> + brw_inst_no_dd_clear(devinfo, inst)), &space);
> +
> + if (devinfo->gen >= 6)
> + err |= qtr_ctrl(file, devinfo, inst);
> + else {
Use braces on the if statement.
> + if (brw_inst_qtr_control(devinfo, inst) == BRW_COMPRESSION_COMPRESSED &&
> + desc && desc->ndst > 0 &&
> + brw_inst_dst_reg_file(devinfo, inst) == BRW_MESSAGE_REGISTER_FILE &&
> + brw_inst_dst_da_reg_nr(devinfo, inst) & BRW_MRF_COMPR4) {
> + format(file, " compr4");
> + } else {
> + err |= control(file, "compression control", compr_ctrl,
> + brw_inst_qtr_control(devinfo, inst), &space);
> + }
> + }
> +
> + err |= control(file, "compaction", cmpt_ctrl, is_compacted, &space);
> + err |= control(file, "thread control", thread_ctrl,
> + brw_inst_thread_control(devinfo, inst), &space);
> + if (has_branch_ctrl(devinfo, opcode)) {
> + err |= control(file, "branch ctrl", branch_ctrl,
> + brw_inst_branch_control(devinfo, inst), &space);
> + } else if (devinfo->gen >= 6) {
> + err |= control(file, "acc write control", accwr,
> + brw_inst_acc_wr_control(devinfo, inst), &space);
> + }
> + if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC)
> + err |= control(file, "end of thread", end_of_thread,
> + brw_inst_eot(devinfo, inst), &space);
> + if (space)
> + string(file, " ");
> + string(file, "}");
> + }
> + string(file, ";");
> +
> + if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) {
> + enum brw_message_target sfid = brw_inst_sfid(devinfo, inst);
>
> - newline(file);
> - pad(file, 16);
> space = 0;
>
> - fprintf(file, " ");
> + fprintf(file, " // ");
> err |= control(file, "SFID", devinfo->gen >= 6 ? gen6_sfid : gen4_sfid,
> sfid, &space);
>
>
> - if (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)) {
> format(file, " indirect");
> } else {
> switch (sfid) {
> @@ -1590,56 +1676,19 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
> format(file, "mlen %"PRIu64, brw_inst_mlen(devinfo, inst));
> format(file, " rlen %"PRIu64, brw_inst_rlen(devinfo, inst));
> }
> - }
> - pad(file, 64);
> - if (opcode != BRW_OPCODE_NOP && opcode != BRW_OPCODE_NENOP) {
> - string(file, "{");
> - space = 1;
> - err |= control(file, "access mode", access_mode,
> - brw_inst_access_mode(devinfo, inst), &space);
> - if (devinfo->gen >= 6) {
> - err |= control(file, "write enable control", wectrl,
> - brw_inst_mask_control(devinfo, inst), &space);
> - } else {
> - err |= control(file, "mask control", mask_ctrl,
> - brw_inst_mask_control(devinfo, inst), &space);
> - }
> - err |= control(file, "dependency control", dep_ctrl,
> - ((brw_inst_no_dd_check(devinfo, inst) << 1) |
> - brw_inst_no_dd_clear(devinfo, inst)), &space);
> -
> - if (devinfo->gen >= 6)
> - err |= qtr_ctrl(file, devinfo, inst);
> - else {
> - if (brw_inst_qtr_control(devinfo, inst) == BRW_COMPRESSION_COMPRESSED &&
> - desc && desc->ndst > 0 &&
> - brw_inst_dst_reg_file(devinfo, inst) == BRW_MESSAGE_REGISTER_FILE &&
> - brw_inst_dst_da_reg_nr(devinfo, inst) & BRW_MRF_COMPR4) {
> - format(file, " compr4");
> - } else {
> - err |= control(file, "compression control", compr_ctrl,
> - brw_inst_qtr_control(devinfo, inst), &space);
> + } else {
> + if (brw_inst_src0_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE) {
> + if (brw_inst_src0_reg_type(devinfo, inst) == BRW_HW_REG_TYPE_F) {
> + fprintf(file, " // %-g", brw_inst_imm_f(devinfo, inst));
> + } else if (brw_inst_src0_reg_type(devinfo, inst) == GEN8_HW_REG_IMM_TYPE_DF) {
> + fprintf(file, " // %-g", brw_inst_imm_df(devinfo, inst));
> }
> + } else if (brw_inst_src1_reg_file(devinfo, inst) == BRW_IMMEDIATE_VALUE &&
> + brw_inst_src1_reg_type(devinfo, inst) == BRW_HW_REG_TYPE_F) {
> + fprintf(file, " // %-g", brw_inst_imm_f(devinfo, inst));
> }
> -
> - err |= control(file, "compaction", cmpt_ctrl, is_compacted, &space);
> - err |= control(file, "thread control", thread_ctrl,
> - brw_inst_thread_control(devinfo, inst), &space);
> - if (has_branch_ctrl(devinfo, opcode)) {
> - err |= control(file, "branch ctrl", branch_ctrl,
> - brw_inst_branch_control(devinfo, inst), &space);
> - } else if (devinfo->gen >= 6) {
> - err |= control(file, "acc write control", accwr,
> - brw_inst_acc_wr_control(devinfo, inst), &space);
> - }
> - if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC)
> - err |= control(file, "end of thread", end_of_thread,
> - brw_inst_eot(devinfo, inst), &space);
> - if (space)
> - string(file, " ");
> - string(file, "}");
> }
> - string(file, ";");
> +
> newline(file);
> return err;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.c b/src/mesa/drivers/dri/i965/brw_eu.c
> index 6a422bb..893de8b 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu.c
> @@ -365,43 +365,126 @@ const unsigned *brw_get_program( struct brw_codegen *p,
> return (const unsigned *)p->store;
> }
>
> +const struct brw_label *
> +brw_find_label(struct brw_label *root, int offset) {
> + struct brw_label *curr = root;
> +
> + if (curr != NULL)
> + {
> + do {
> + if (curr->offset == offset)
> + return curr;
> +
> + curr = curr->next;
> + } while (curr != NULL);
> + }
> +
> + return curr;
> +}
> +
> +void
> +brw_create_label(struct brw_label **labels, int offset, void *mem_ctx) {
> + if (*labels != NULL) {
> + struct brw_label *curr = *labels;
> + struct brw_label *prev;
> +
> + do {
> + prev = curr;
> +
> + if (curr->offset == offset)
> + return;
> +
> + curr = curr->next;
> + } while (curr != NULL);
> +
> + curr = ralloc(mem_ctx, struct brw_label);
> + curr->offset = offset;
> + curr->number = prev->number + 1;
> + curr->next = NULL;
> + prev->next = curr;
> + } else {
> + struct brw_label *root = ralloc(mem_ctx, struct brw_label);
> + root->number = 0;
> + root->offset = offset;
> + root->next = NULL;
> + *labels = root;
> + }
> +}
> +
> void
> brw_disassemble(const struct gen_device_info *devinfo,
> void *assembly, int start, int end, FILE *out)
> {
> bool dump_hex = (INTEL_DEBUG & DEBUG_HEX) != 0;
>
> + void *mem_ctx = ralloc_context(NULL);
> + struct brw_label *root = NULL;
> +
> + if ((INTEL_DEBUG & DEBUG_DISASM) != 0) {
> + for (int offset = start; offset < end;) {
> + brw_inst *insn = assembly + offset;
> + brw_inst uncompacted;
> +
> + bool compacted = brw_inst_cmpt_control(devinfo, insn);
> +
> + if (compacted) {
> + brw_compact_inst *compacted = (void *)insn;
> + brw_uncompact_instruction(devinfo, &uncompacted, compacted);
> + insn = &uncompacted;
> + }
> +
> + if (brw_has_uip(devinfo, brw_inst_opcode(devinfo, insn)))
> + brw_create_label(&root, offset + brw_inst_uip(devinfo, insn), mem_ctx);
> +
> + if (brw_has_jip(devinfo, brw_inst_opcode(devinfo, insn)))
> + brw_create_label(&root, offset + brw_inst_jip(devinfo, insn), mem_ctx);
> +
> + offset += compacted ? 8 : 16;
> + }
> + }
> +
> for (int offset = start; offset < end;) {
> brw_inst *insn = assembly + offset;
> brw_inst uncompacted;
> +
> + if (root != NULL) {
> + const struct brw_label *label = brw_find_label(root, offset);
> + if (label != NULL) {
> + fprintf(out, "\n_label%d:\n", label->number);
> + }
> + }
> +
> bool compacted = brw_inst_cmpt_control(devinfo, insn);
> +
> if (0)
> fprintf(out, "0x%08x: ", offset);
>
> if (compacted) {
> brw_compact_inst *compacted = (void *)insn;
> - if (dump_hex) {
> - fprintf(out, "0x%08x 0x%08x ",
> - ((uint32_t *)insn)[1],
> - ((uint32_t *)insn)[0]);
> - }
> -
> - brw_uncompact_instruction(devinfo, &uncompacted, compacted);
> - insn = &uncompacted;
> - offset += 8;
> +
> + if (dump_hex) {
> + fprintf(out, "0x%08x 0x%08x ",
> + ((uint32_t *)insn)[1],
> + ((uint32_t *)insn)[0]);
> + }
> +
> + brw_uncompact_instruction(devinfo, &uncompacted, compacted);
> + insn = &uncompacted;
> } else {
> - if (dump_hex) {
> - fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ",
> - ((uint32_t *)insn)[3],
> - ((uint32_t *)insn)[2],
> - ((uint32_t *)insn)[1],
> - ((uint32_t *)insn)[0]);
> - }
> - offset += 16;
> + if (dump_hex) {
> + fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ",
> + ((uint32_t *)insn)[3],
> + ((uint32_t *)insn)[2],
> + ((uint32_t *)insn)[1],
> + ((uint32_t *)insn)[0]);
Arguments are not indented properly.
> + }
> }
>
> - brw_disassemble_inst(out, devinfo, insn, compacted);
> + brw_disassemble_inst(out, devinfo, insn, compacted, offset, root);
> + offset += compacted ? 8 : 16;
> }
> +
> + ralloc_free(mem_ctx);
> }
>
> enum gen {
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h
> index c44896b..9f49eb7 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -94,6 +94,12 @@ struct brw_codegen {
> int loop_stack_array_size;
> };
>
> +struct brw_label {
> + int offset;
> + int number;
> + struct brw_label *next;
> +};
> +
> void brw_pop_insn_state( struct brw_codegen *p );
> void brw_push_insn_state( struct brw_codegen *p );
> void brw_set_default_exec_size(struct brw_codegen *p, unsigned value);
> @@ -114,6 +120,10 @@ void brw_set_default_acc_write_control(struct brw_codegen *p, unsigned value);
>
> void brw_init_codegen(const struct gen_device_info *, struct brw_codegen *p,
> void *mem_ctx);
> +bool brw_has_jip(const struct gen_device_info *devinfo, enum opcode opcode);
> +bool brw_has_uip(const struct gen_device_info *devinfo, enum opcode opcode);
> +const struct brw_label *brw_find_label(struct brw_label *root, int offset);
> +void brw_create_label(struct brw_label **labels, int offset, void *mem_ctx);
> void brw_disassemble(const struct gen_device_info *devinfo, void *assembly,
> int start, int end, FILE *out);
> const unsigned *brw_get_program( struct brw_codegen *p, unsigned *sz );
> @@ -121,6 +131,7 @@ const unsigned *brw_get_program( struct brw_codegen *p, unsigned *sz );
> brw_inst *brw_next_insn(struct brw_codegen *p, unsigned opcode);
> void brw_set_dest(struct brw_codegen *p, brw_inst *insn, struct brw_reg dest);
> void brw_set_src0(struct brw_codegen *p, brw_inst *insn, struct brw_reg reg);
> +void brw_set_src1(struct brw_codegen *p, brw_inst *insn, struct brw_reg reg);
I see the prototype for brw_set_src1 is in an unexpected place in
brw_eu.h. Moving it here would be fine, but not in this patch.
>
> void gen6_resolve_implied_move(struct brw_codegen *p,
> struct brw_reg *src,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> index 68321e7..d20c6d1 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> @@ -1262,10 +1262,10 @@ void brw_debug_compact_uncompact(const struct gen_device_info *devinfo,
> devinfo->gen);
>
> fprintf(stderr, " before: ");
> - brw_disassemble_inst(stderr, devinfo, orig, true);
> + brw_disassemble_inst(stderr, devinfo, orig, true, 0, NULL);
>
> fprintf(stderr, " after: ");
> - brw_disassemble_inst(stderr, devinfo, uncompacted, false);
> + brw_disassemble_inst(stderr, devinfo, uncompacted, false, 0, NULL);
>
> uint32_t *before_bits = (uint32_t *)orig;
> uint32_t *after_bits = (uint32_t *)uncompacted;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 26ffbb1..79ff76b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -2101,8 +2101,12 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
> spill_count, fill_count, promoted_constants, before_size, after_size,
> 100.0f * (before_size - after_size) / before_size);
>
> - dump_assembly(p->store, annotation.ann_count, annotation.ann,
> - p->devinfo);
> + if ((INTEL_DEBUG & DEBUG_DISASM) != 0)
> + brw_disassemble(p->devinfo, p->store, start_offset, start_offset + after_size, stderr);
> + else
> + dump_assembly(p->store, annotation.ann_count, annotation.ann,
> + p->devinfo);
> +
> ralloc_free(annotation.mem_ctx);
> }
> assert(validated);
> diff --git a/src/mesa/drivers/dri/i965/intel_debug.c b/src/mesa/drivers/dri/i965/intel_debug.c
> index 33e8402..3d0bb62 100644
> --- a/src/mesa/drivers/dri/i965/intel_debug.c
> +++ b/src/mesa/drivers/dri/i965/intel_debug.c
> @@ -81,6 +81,7 @@ static const struct debug_control debug_control[] = {
> { "l3", DEBUG_L3 },
> { "do32", DEBUG_DO32 },
> { "norbc", DEBUG_NO_RBC },
> + { "disasm", DEBUG_DISASM },
> { NULL, 0 }
> };
>
> diff --git a/src/mesa/drivers/dri/i965/intel_debug.h b/src/mesa/drivers/dri/i965/intel_debug.h
> index afca36e..191b9e4 100644
> --- a/src/mesa/drivers/dri/i965/intel_debug.h
> +++ b/src/mesa/drivers/dri/i965/intel_debug.h
> @@ -73,6 +73,7 @@ extern uint64_t INTEL_DEBUG;
> #define DEBUG_L3 (1ull << 37)
> #define DEBUG_DO32 (1ull << 38)
> #define DEBUG_NO_RBC (1ull << 39)
> +#define DEBUG_DISASM (1ull << 40)
>
> #ifdef HAVE_ANDROID_PLATFORM
> #define LOG_TAG "INTEL-MESA"
> diff --git a/src/mesa/drivers/dri/i965/test_eu_compact.c b/src/mesa/drivers/dri/i965/test_eu_compact.c
> index 874412c..aa9cd99 100644
> --- a/src/mesa/drivers/dri/i965/test_eu_compact.c
> +++ b/src/mesa/drivers/dri/i965/test_eu_compact.c
> @@ -49,7 +49,7 @@ test_compact_instruction(struct brw_codegen *p, brw_inst src)
> if (memcmp(&unchanged, &dst, sizeof(dst))) {
> fprintf(stderr, "Failed to compact, but dst changed\n");
> fprintf(stderr, " Instruction: ");
> - brw_disassemble_inst(stderr, p->devinfo, &src, false);
> + (stderr, p->devinfo, &src, false, NULL);
> return false;
> }
> }
> --
> 2.7.4
Okay, there's a bunch of stuff going on here.
Please remove the unrelated changes. That'll reduce the noise quite a bit.
Write a commit message that explains what's going on. That'll help the
reviewer to start on the same page.
I think it would be good to split the SFID changes out into their own patch.
More information about the mesa-dev
mailing list