[Mesa-dev] [PATCH 11/11] intel/tools/error: Decode compute shaders.

[Lionel Landwerlin]

On 13/11/17 21:40, Kenneth Graunke wrote:
> On Sunday, November 12, 2017 4:06:06 AM PST Lionel Landwerlin wrote:
>> On 12/11/17 08:35, Kenneth Graunke wrote:
>>> This is a bit more annoying than your average shader - we need to look
>>> at MEDIA_INTERFACE_DESCRIPTOR_LOAD in the batch buffer, then hop over
>>> to the dynamic state buffer to read the INTERFACE_DESCRIPTOR_DATA, then
>>> hop over to the instruction buffer to decode the program.
>>>
>>> Now that we store all the buffers before decoding, we can actually do
>>> this fairly easily.
>>> ---
>>>    src/intel/tools/aubinator_error_decode.c | 49 +++++++++++++++++++++++++++-----
>>>    1 file changed, 42 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
>>> index 09ca7c3a4ab..81418d69c9a 100644
>>> --- a/src/intel/tools/aubinator_error_decode.c
>>> +++ b/src/intel/tools/aubinator_error_decode.c
>>> @@ -233,6 +233,17 @@ disassemble_program(struct gen_disasm *disasm, const char *type,
>>>       gen_disasm_disassemble(disasm, instruction_section->data, ksp, stdout);
>>>    }
>>>    
>>> +static const struct section *
>>> +find_section(const char *str_base_address)
>>> +{
>>> +   uint64_t base_address = strtol(str_base_address, NULL, 16);
>>> +   for (int s = 0; s < MAX_SECTIONS; s++) {
>>> +      if (sections[s].gtt_offset == base_address)
>>> +         return &sections[s];
>>> +   }
>>> +   return NULL;
>>> +}
>>> +
>>>    static void
>>>    decode(struct gen_spec *spec, struct gen_disasm *disasm,
>>>           const struct section *section)
>>> @@ -243,6 +254,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>>>       int length;
>>>       struct gen_group *inst;
>>>       const struct section *current_instruction_buffer = NULL;
>>> +   const struct section *current_dynamic_state_buffer = NULL;
>>>    
>>>       for (p = data; p < end; p += length) {
>>>          const char *color = option_full_decode ? BLUE_HEADER : NORMAL,
>>> @@ -277,13 +289,9 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>>>    
>>>             do {
>>>                if (strcmp(iter.name, "Instruction Base Address") == 0) {
>>> -               uint64_t instr_base_address = strtol(iter.value, NULL, 16);
>>> -               current_instruction_buffer = NULL;
>>> -               for (int s = 0; s < MAX_SECTIONS; s++) {
>>> -                  if (sections[s].gtt_offset == instr_base_address) {
>>> -                     current_instruction_buffer = &sections[s];
>>> -                  }
>>> -               }
>>> +               current_instruction_buffer = find_section(iter.value);
>>> +            } else if (strcmp(iter.name, "Dynamic State Base Address") == 0) {
>>> +               current_dynamic_state_buffer = find_section(iter.value);
>>>                }
>>>             } while (gen_field_iterator_next(&iter));
>>>          } else if (strcmp(inst->name,   "WM_STATE") == 0 ||
>>> @@ -380,6 +388,33 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>>>                disassemble_program(disasm, type, current_instruction_buffer, ksp);
>>>                printf("\n");
>>>             }
>>> +      } else if (strcmp(inst->name, "MEDIA_INTERFACE_DESCRIPTOR_LOAD") == 0) {
>>> +         struct gen_field_iterator iter;
>>> +         gen_field_iterator_init(&iter, inst, p, false);
>>> +         uint64_t interface_offset = 0;
>>> +         while (gen_field_iterator_next(&iter)) {
>> Don't want a do {} while() here too?
> Oops, yes we do.  I'd written this, discovered the do-while problem,
> rebased, wrote a separate patch to fix the earlier problems...but didn't
> fix this one because it didn't exist yet...then forgot.
>
> Fixed locally.  Look okay other than that?

Sure,

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>