On Tue, May 2, 2017 at 6:03 AM, Iago Toral <ito...@igalia.com> wrote:
> On Mon, 2017-05-01 at 13:54 -0700, Matt Turner wrote:
>> Which will allow us to print validation errors found in shader
>> assembly
>> in GPU hang error states.
>> ---
>> src/intel/tools/disasm.c | 71 +++++++++++++++++++++++++++++---------
>> ----------
>> 1 file changed, 43 insertions(+), 28 deletions(-)
>>
>> diff --git a/src/intel/tools/disasm.c b/src/intel/tools/disasm.c
>> index 62256d2..361885b 100644
>> --- a/src/intel/tools/disasm.c
>> +++ b/src/intel/tools/disasm.c
>> @@ -43,52 +43,67 @@ is_send(uint32_t opcode)
>> opcode == BRW_OPCODE_SENDSC );
>> }
>>
>> -void
>> -gen_disasm_disassemble(struct gen_disasm *disasm, void *assembly,
>> - int start, FILE *out)
>> +static int
>> +gen_disasm_find_end(struct gen_disasm *disasm, void *assembly, int
>> start)
>> {
>> struct gen_device_info *devinfo = &disasm->devinfo;
>> - bool dump_hex = false;
>> int offset = start;
>>
>> /* This loop exits when send-with-EOT or when opcode is 0 */
>> while (true) {
>> brw_inst *insn = assembly + offset;
>> - brw_inst uncompacted;
>> - 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;
>> +
>> + if (brw_inst_cmpt_control(devinfo, insn)) {
>> 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]);
>> - }
>> offset += 16;
>> }
>>
>> - brw_disassemble_inst(out, devinfo, insn, compacted);
>> -
>> /* Simplistic, but efficient way to terminate disasm */
>> uint32_t opcode = brw_inst_opcode(devinfo, insn);
>> if (opcode == 0 || (is_send(opcode) && brw_inst_eot(devinfo,
>> insn))) {
>> break;
>> }
>> }
>> +
>> + return offset;
>> +}
>> +
>> +void
>> +gen_disasm_disassemble(struct gen_disasm *disasm, void *assembly,
>> + int start, FILE *out)
>> +{
>> + struct gen_device_info *devinfo = &disasm->devinfo;
>> + int end = gen_disasm_find_end(disasm, assembly, start);
>> +
>> + /* Make a dummy annotation structure that
>> brw_validate_instructions
>> + * can work from.
>> + */
>> + struct annotation_info annotation_info = {
>> + .ann_count = 1,
>> + .ann_size = 2,
>> + };
>> + annotation_info.mem_ctx = ralloc_context(NULL);
>> + annotation_info.ann = rzalloc_array(annotation_info.mem_ctx,
>> + struct annotation,
>> + annotation_info.ann_size);
>> + annotation_info.ann[0].offset = start;
>> + annotation_info.ann[1].offset = end;
>> + brw_validate_instructions(devinfo, assembly, start, end,
>> &annotation_info);
>> + struct annotation *annotation = annotation_info.ann;
>> +
>> + for (int i = 0; i < annotation_info.ann_count; i++) {
>> + int start_offset = annotation[i].offset;
>> + int end_offset = annotation[i + 1].offset;
>
> I was going to say that this code seems to overflow when i
> == annotation_info.ann_count - 1, but then I noticed that there are
> similar loops in other parts of the code and that you initialized
> ann_count to just 1 even though you have two initial annotations, so I
> guess this is how this is supposed to work, right?
Yeah, exactly. I'm not sure it's a good way to handle it though. To be
honest, I had a hard time remembering how it was supposed to work and
I wrote the annotation system.
I think a linked list would be a lot more natural than the array/count.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
