On Thu, May 04, 2017 at 11:27:57AM -0700, Matt Turner wrote:
> 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.
Sounds like it. While not ideal I think we can live with this for now:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
