[Mesa-dev] [PATCH 11/11] intel/aubinator_error_decode: Disassemble shader programs
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Fri May 5 07:44:27 UTC 2017
On Mon, May 01, 2017 at 01:54:55PM -0700, Matt Turner wrote:
> ---
> src/intel/Makefile.tools.am | 6 +-
> src/intel/tools/aubinator_error_decode.c | 178 ++++++++++++++++++++++++++++++-
> 2 files changed, 180 insertions(+), 4 deletions(-)
I had a few nits but the logic looks right to me. Thanks for working on this,
I hope we can catch some of the hangs in CI now!
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>
> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
> index 576beea..1175118 100644
> --- a/src/intel/Makefile.tools.am
> +++ b/src/intel/Makefile.tools.am
> @@ -47,11 +47,15 @@ tools_aubinator_LDADD = \
>
>
> tools_aubinator_error_decode_SOURCES = \
> - tools/aubinator_error_decode.c
> + tools/aubinator_error_decode.c \
> + tools/disasm.c \
> + tools/gen_disasm.h
>
> tools_aubinator_error_decode_LDADD = \
> common/libintel_common.la \
> + compiler/libintel_compiler.la \
> $(top_builddir)/src/util/libmesautil.la \
> + $(PTHREAD_LIBS) \
> $(EXPAT_LIBS) \
> $(ZLIB_LIBS)
>
> diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
> index 244bef8..ef77c01 100644
> --- a/src/intel/tools/aubinator_error_decode.c
> +++ b/src/intel/tools/aubinator_error_decode.c
> @@ -40,6 +40,7 @@
>
> #include "common/gen_decoder.h"
> #include "util/macros.h"
> +#include "gen_disasm.h"
>
> #define CSI "\e["
> #define BLUE_HEADER CSI "0;44m"
> @@ -209,6 +210,17 @@ print_fault_data(struct gen_device_info *devinfo, uint32_t data1, uint32_t data0
> #define CSI "\e["
> #define NORMAL CSI "0m"
>
> +struct program {
> + const char *type;
> + const char *command;
> + uint64_t command_offset;
> + uint64_t instruction_base_address;
> + uint64_t ksp;
> +};
> +
> +static struct program programs[4096];
How about?
#define MAX_NUM_PROGRAMS 4096
static struct program programs[MAX_NUM_PROGRAMS];
And guard against overflowing in the iterator further down? Just in case
input is somehow corrupted.
> +static int num_programs = 0;
> +
> static void decode(struct gen_spec *spec,
> const char *buffer_name,
> const char *ring_name,
> @@ -219,6 +231,7 @@ static void decode(struct gen_spec *spec,
> uint32_t *p, *end = (data + *count);
> int length;
> struct gen_group *inst;
> + uint64_t current_instruction_base_address = 0;
>
> for (p = data; p < end; p += length) {
> const char *color = option_full_decode ? BLUE_HEADER : NORMAL,
> @@ -246,6 +259,127 @@ static void decode(struct gen_spec *spec,
>
> if (strcmp(inst->name, "MI_BATCH_BUFFER_END") == 0)
> break;
> +
> + if (strcmp(inst->name, "STATE_BASE_ADDRESS") == 0) {
> + struct gen_field_iterator iter;
> + gen_field_iterator_init(&iter, inst, p, false);
> +
> + while (gen_field_iterator_next(&iter)) {
> + if (strcmp(iter.name, "Instruction Base Address") == 0) {
> + current_instruction_base_address = strtol(iter.value, NULL, 16);
> + }
> + }
> + } else if (strcmp(inst->name, "WM_STATE") == 0 ||
> + strcmp(inst->name, "3DSTATE_PS") == 0 ||
> + strcmp(inst->name, "3DSTATE_WM") == 0) {
> + struct gen_field_iterator iter;
> + gen_field_iterator_init(&iter, inst, p, false);
> + uint64_t ksp[3] = {0, 0, 0};
> + bool enabled[3] = {false, false, false};
> +
> + while (gen_field_iterator_next(&iter)) {
> + if (strncmp(iter.name, "Kernel Start Pointer ",
> + strlen("Kernel Start Pointer ")) == 0) {
> + int idx = iter.name[strlen("Kernel Start Pointer ")] - '0';
> + ksp[idx] = strtol(iter.value, NULL, 16);
> + } else if (strcmp(iter.name, "8 Pixel Dispatch Enable") == 0) {
> + enabled[0] = strcmp(iter.value, "true") == 0;
> + } else if (strcmp(iter.name, "16 Pixel Dispatch Enable") == 0) {
> + enabled[1] = strcmp(iter.value, "true") == 0;
> + } else if (strcmp(iter.name, "32 Pixel Dispatch Enable") == 0) {
> + enabled[2] = strcmp(iter.value, "true") == 0;
> + }
> + }
> +
> + /* FINISHME: Broken for multi-program WM_STATE,
> + * which Mesa does not use
> + */
> + if (enabled[0] + enabled[1] + enabled[2] == 1) {
> + const char *type = enabled[0] ? "SIMD8 fragment shader" :
> + enabled[1] ? "SIMD16 fragment shader" :
> + enabled[2] ? "SIMD32 fragment shader" : NULL;
> +
> + programs[num_programs++] = (struct program) {
> + .type = type,
> + .command = inst->name,
> + .command_offset = offset,
> + .instruction_base_address = current_instruction_base_address,
> + .ksp = ksp[0],
> + };
> + } else {
> + if (enabled[0]) /* SIMD8 */ {
> + programs[num_programs++] = (struct program) {
> + .type = "SIMD8 fragment shader",
> + .command = inst->name,
> + .command_offset = offset,
> + .instruction_base_address = current_instruction_base_address,
> + .ksp = ksp[0],
> + .ksp = ksp[0], /* SIMD8 shader is specified by ksp[0] */
> + };
> + }
> + if (enabled[1]) /* SIMD16 */ {
> + programs[num_programs++] = (struct program) {
> + .type = "SIMD16 fragment shader",
> + .command = inst->name,
> + .command_offset = offset,
> + .instruction_base_address = current_instruction_base_address,
> + .ksp = ksp[2], /* SIMD16 shader is specified by ksp[2] */
> + };
> + }
> + if (enabled[2]) /* SIMD32 */ {
> + programs[num_programs++] = (struct program) {
> + .type = "SIMD32 fragment shader",
> + .command = inst->name,
> + .command_offset = offset,
> + .instruction_base_address = current_instruction_base_address,
> + .ksp = ksp[1], /* SIMD32 shader is specified by ksp[1] */
> + };
> + }
> + }
> + } else if (strcmp(inst->name, "VS_STATE") == 0 ||
> + strcmp(inst->name, "GS_STATE") == 0 ||
> + strcmp(inst->name, "SF_STATE") == 0 ||
> + strcmp(inst->name, "CLIP_STATE") == 0 ||
> + strcmp(inst->name, "3DSTATE_DS") == 0 ||
> + strcmp(inst->name, "3DSTATE_HS") == 0 ||
> + strcmp(inst->name, "3DSTATE_GS") == 0 ||
> + strcmp(inst->name, "3DSTATE_VS") == 0) {
> + struct gen_field_iterator iter;
> + gen_field_iterator_init(&iter, inst, p, false);
> + uint64_t ksp;
In case of fragment programs you initialise to zero, should we do it here
also? Otherwise there is possibility that we fill random value to program
list below. (I suppose input would need to be invalid again...).
> + bool is_simd8 = false; /* vertex shaders on Gen8+ only */
> + bool is_enabled = true;
And shouldn't this be 'false' by default?
> +
> + while (gen_field_iterator_next(&iter)) {
> + if (strcmp(iter.name, "Kernel Start Pointer") == 0) {
> + ksp = strtol(iter.value, NULL, 16);
> + } else if (strcmp(iter.name, "SIMD8 Dispatch Enable") == 0) {
> + is_simd8 = strcmp(iter.value, "true") == 0;
> + } else if (strcmp(iter.name, "Function Enable") == 0) {
> + is_enabled = strcmp(iter.value, "true") == 0;
> + }
> + }
> +
> + const char *type = strcmp(inst->name, "VS_STATE") == 0 ? "vertex shader" :
> + strcmp(inst->name, "GS_STATE") == 0 ? "geometry shader" :
> + strcmp(inst->name, "SF_STATE") == 0 ? "strips and fans shader" :
> + strcmp(inst->name, "CLIP_STATE") == 0 ? "clip shader" :
> + strcmp(inst->name, "3DSTATE_DS") == 0 ? "tessellation control shader" :
> + strcmp(inst->name, "3DSTATE_HS") == 0 ? "tessellation evaluation shader" :
> + strcmp(inst->name, "3DSTATE_GS") == 0 ? "geometry shader" :
> + strcmp(inst->name, "3DSTATE_VS") == 0 ? is_simd8 ?
> + "SIMD8 vertex shader" : "vec4 vertex shader" : NULL;
> +
> + if (is_enabled) {
> + programs[num_programs++] = (struct program) {
> + .type = type,
> + .command = inst->name,
> + .command_offset = offset,
> + .instruction_base_address = current_instruction_base_address,
> + .ksp = ksp,
> + };
> + }
> + }
> }
> }
>
> @@ -348,6 +482,7 @@ read_data_file(FILE *file)
> const char *buffer_name = "batch buffer";
> char *ring_name = NULL;
> struct gen_device_info devinfo;
> + struct gen_disasm *disasm = NULL;
>
> while (getline(&line, &line_size, file) > 0) {
> char *new_ring_name = NULL;
> @@ -424,6 +559,22 @@ read_data_file(FILE *file)
> buffer_name = "HW Context";
> continue;
> }
> +
> + matched = sscanf(dashes, "--- user = 0x%08x %08x\n",
> + &hi, &lo);
> + if (matched > 0) {
> + new_gtt_offset = hi;
> + if (matched == 2) {
> + new_gtt_offset <<= 32;
> + new_gtt_offset |= lo;
> + }
> +
> + gtt_offset = new_gtt_offset;
> + free(ring_name);
> + ring_name = new_ring_name;
> + buffer_name = "user";
> + continue;
> + }
Looks like this is now third copy of almost identical block. Perhaps later
on we could simplify this a little. No strong feeling though.
> }
>
> if (line[0] == ':' || line[0] == '~') {
> @@ -432,9 +583,27 @@ read_data_file(FILE *file)
> fprintf(stderr, "ASCII85 decode failed.\n");
> exit(1);
> }
> - decode(spec,
> - buffer_name, ring_name,
> - gtt_offset, data, &count);
> +
> + if (strcmp(buffer_name, "user") == 0) {
> + printf("Disassembly of programs in instruction buffer at "
> + "0x%08"PRIx64":\n", gtt_offset);
> + for (int i = 0; i < num_programs; i++) {
> + if (programs[i].instruction_base_address == gtt_offset) {
> + printf("\n%s (specified by %s at batch offset "
> + "0x%08"PRIx64") at offset 0x%08"PRIx64"\n",
> + programs[i].type,
> + programs[i].command,
> + programs[i].command_offset,
> + programs[i].ksp);
> + gen_disasm_disassemble(disasm, data, programs[i].ksp,
> + stdout);
> + }
> + }
> + } else {
> + decode(spec,
> + buffer_name, ring_name,
> + gtt_offset, data, &count);
> + }
> continue;
> }
>
> @@ -463,6 +632,8 @@ read_data_file(FILE *file)
> return;
> }
>
> + disasm = gen_disasm_create(reg);
> +
> printf("Detected GEN%i chipset\n", devinfo.gen);
>
> if (xml_path == NULL)
> @@ -552,6 +723,7 @@ read_data_file(FILE *file)
> buffer_name, ring_name,
> gtt_offset, data, &count);
>
> + gen_disasm_destroy(disasm);
> free(data);
> free(line);
> free(ring_name);
> --
> 2.10.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list