[Mesa-dev] [PATCH 1/3] Added label support for shader disassembly. SEND and SENDC formatting now also includes the SFID information.
Toni Lönnberg
toni.lonnberg at intel.com
Mon Feb 13 14:56:46 UTC 2017
First patch I've ever submitted so bound to be some mistakes here and
there :)
I would've thought the name "label" wouldn't have any ambiguity as it's
been used extensively in assembler coding. But yes, it's just what you
think it is, a jump target. Currently Mesa's disassembly dump uses
relative immediate values in halt/if/else instructionss which makes it a
bit hard to actually see where the code is jumping to.
What kind of background information would you like to have in the commit
message?
-Toni
On 13.02.2017 15:31, Matt Turner wrote:
> 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.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
More information about the mesa-dev
mailing list