[Mesa-dev] [PATCH 3/4] i965: Rewrite disassembly annotation code

Samuel Iglesias Gonsálvez siglesias at igalia.com
Fri Nov 17 10:14:50 UTC 2017


You can add my Acked-by to this patch, I skimmed over it and I have not
found any error.

Acked-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>

I would be better for review if it was split in different patches, even
though they would be squashed together before pushing to master.

Sam

On Thu, 2017-11-16 at 20:47 -0800, Matt Turner wrote:
> The old code used an array to store each "instruction group" (the
> new,
> better name than the old overloaded "annotation"), and required a
> memmove() to shift elements over in the array when we needed to split
> a
> group so that we could add an error message. This was confusing and
> difficult to get right, not the least of which was  because the array
> has a tail sentinel not included in .ann_count.
> 
> Instead use a linked list, a data structure made for efficient
> insertion.
> ---
> I'm admittedly doing a fair number of things in this patch. If
> requested,
> I can attempt to split it up into smaller pieces.
> 
>  src/intel/compiler/brw_compile_clip.c     |   2 +-
>  src/intel/compiler/brw_eu.h               |   4 +-
>  src/intel/compiler/brw_eu_compact.c       |  26 +++--
>  src/intel/compiler/brw_eu_validate.c      |   6 +-
>  src/intel/compiler/brw_fs_generator.cpp   |  21 ++--
>  src/intel/compiler/brw_vec4_generator.cpp |  18 ++--
>  src/intel/compiler/intel_asm_annotation.c | 173 +++++++++++++++-----
> ----------
>  src/intel/compiler/intel_asm_annotation.h |  38 ++++---
>  src/intel/compiler/test_eu_validate.cpp   |  17 ++-
>  src/intel/tools/disasm.c                  |  40 +++----
>  src/mesa/drivers/dri/i965/brw_ff_gs.c     |   2 +-
>  11 files changed, 177 insertions(+), 170 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_compile_clip.c
> b/src/intel/compiler/brw_compile_clip.c
> index 83788e4b64..c04d1a8277 100644
> --- a/src/intel/compiler/brw_compile_clip.c
> +++ b/src/intel/compiler/brw_compile_clip.c
> @@ -79,7 +79,7 @@ brw_compile_clip(const struct brw_compiler
> *compiler,
>        unreachable("not reached");
>     }
>  
> -   brw_compact_instructions(&c.func, 0, 0, NULL);
> +   brw_compact_instructions(&c.func, 0, NULL);
>  
>     *prog_data = c.prog_data;
>  
> diff --git a/src/intel/compiler/brw_eu.h
> b/src/intel/compiler/brw_eu.h
> index 95503d5513..d66988da56 100644
> --- a/src/intel/compiler/brw_eu.h
> +++ b/src/intel/compiler/brw_eu.h
> @@ -548,7 +548,7 @@ enum brw_conditional_mod brw_swap_cmod(uint32_t
> cmod);
>  /* brw_eu_compact.c */
>  void brw_init_compaction_tables(const struct gen_device_info
> *devinfo);
>  void brw_compact_instructions(struct brw_codegen *p, int
> start_offset,
> -                              int num_annotations, struct annotation
> *annotation);
> +                              struct disasm_info *disasm);
>  void brw_uncompact_instruction(const struct gen_device_info
> *devinfo,
>                                 brw_inst *dst, brw_compact_inst
> *src);
>  bool brw_try_compact_instruction(const struct gen_device_info
> *devinfo,
> @@ -560,7 +560,7 @@ void brw_debug_compact_uncompact(const struct
> gen_device_info *devinfo,
>  /* brw_eu_validate.c */
>  bool brw_validate_instructions(const struct gen_device_info
> *devinfo,
>                                 const void *assembly, int
> start_offset, int end_offset,
> -                               struct annotation_info *annotation);
> +                               struct disasm_info *disasm);
>  
>  static inline int
>  next_offset(const struct gen_device_info *devinfo, void *store, int
> offset)
> diff --git a/src/intel/compiler/brw_eu_compact.c
> b/src/intel/compiler/brw_eu_compact.c
> index a9da46957a..3a3875356d 100644
> --- a/src/intel/compiler/brw_eu_compact.c
> +++ b/src/intel/compiler/brw_eu_compact.c
> @@ -1486,7 +1486,7 @@ brw_init_compaction_tables(const struct
> gen_device_info *devinfo)
>  
>  void
>  brw_compact_instructions(struct brw_codegen *p, int start_offset,
> -                         int num_annotations, struct annotation
> *annotation)
> +                         struct disasm_info *disasm)
>  {
>     if (unlikely(INTEL_DEBUG & DEBUG_NO_COMPACTION))
>        return;
> @@ -1501,7 +1501,7 @@ brw_compact_instructions(struct brw_codegen *p,
> int start_offset,
>     /* For an instruction at byte offset 8*i after compaction, this
> was its IP
>      * (in 16-byte units) before compaction.
>      */
> -   int old_ip[(p->next_insn_offset - start_offset) /
> sizeof(brw_compact_inst)];
> +   int old_ip[(p->next_insn_offset - start_offset) /
> sizeof(brw_compact_inst) + 1];
>  
>     if (devinfo->gen == 4 && !devinfo->is_g4x)
>        return;
> @@ -1556,6 +1556,12 @@ brw_compact_instructions(struct brw_codegen
> *p, int start_offset,
>        }
>     }
>  
> +   /* Add an entry for the ending offset of the program. This
> greatly
> +    * simplifies the linked list walk at the end of the function.
> +    */
> +   old_ip[offset / sizeof(brw_compact_inst)] =
> +      (p->next_insn_offset - start_offset) / sizeof(brw_inst);
> +
>     /* Fix up control flow offsets. */
>     p->next_insn_offset = start_offset + offset;
>     for (offset = 0; offset < p->next_insn_offset - start_offset;
> @@ -1651,21 +1657,21 @@ brw_compact_instructions(struct brw_codegen
> *p, int start_offset,
>     }
>     p->nr_insn = p->next_insn_offset / sizeof(brw_inst);
>  
> -   /* Update the instruction offsets for each annotation. */
> -   if (annotation) {
> -      for (int offset = 0, i = 0; i < num_annotations; i++) {
> +   /* Update the instruction offsets for each group. */
> +   if (disasm) {
> +      int offset = 0;
> +
> +      foreach_list_typed(struct inst_group, group, link, disasm-
> >group_list) {
>           while (start_offset + old_ip[offset /
> sizeof(brw_compact_inst)] *
> -                sizeof(brw_inst) != annotation[i].offset) {
> +                sizeof(brw_inst) != group->offset) {
>              assert(start_offset + old_ip[offset /
> sizeof(brw_compact_inst)] *
> -                   sizeof(brw_inst) < annotation[i].offset);
> +                   sizeof(brw_inst) < group->offset);
>              offset = next_offset(devinfo, store, offset);
>           }
>  
> -         annotation[i].offset = start_offset + offset;
> +         group->offset = start_offset + offset;
>  
>           offset = next_offset(devinfo, store, offset);
>        }
> -
> -      annotation[num_annotations].offset = p->next_insn_offset;
>     }
>  }
> diff --git a/src/intel/compiler/brw_eu_validate.c
> b/src/intel/compiler/brw_eu_validate.c
> index 647835af7a..6ee6b4ffbe 100644
> --- a/src/intel/compiler/brw_eu_validate.c
> +++ b/src/intel/compiler/brw_eu_validate.c
> @@ -1258,7 +1258,7 @@
> special_requirements_for_handling_double_precision_data_types(
>  bool
>  brw_validate_instructions(const struct gen_device_info *devinfo,
>                            const void *assembly, int start_offset,
> int end_offset,
> -                          struct annotation_info *annotation)
> +                          struct disasm_info *disasm)
>  {
>     bool valid = true;
>  
> @@ -1286,8 +1286,8 @@ brw_validate_instructions(const struct
> gen_device_info *devinfo,
>           CHECK(special_requirements_for_handling_double_precision_da
> ta_types);
>        }
>  
> -      if (error_msg.str && annotation) {
> -         annotation_insert_error(annotation, src_offset,
> error_msg.str);
> +      if (error_msg.str && disasm) {
> +         disasm_insert_error(disasm, src_offset, error_msg.str);
>        }
>        valid = valid && error_msg.len == 0;
>        free(error_msg.str);
> diff --git a/src/intel/compiler/brw_fs_generator.cpp
> b/src/intel/compiler/brw_fs_generator.cpp
> index 96691ac3ff..434a40df15 100644
> --- a/src/intel/compiler/brw_fs_generator.cpp
> +++ b/src/intel/compiler/brw_fs_generator.cpp
> @@ -1638,8 +1638,7 @@ fs_generator::generate_code(const cfg_t *cfg,
> int dispatch_width)
>     int spill_count = 0, fill_count = 0;
>     int loop_count = 0;
>  
> -   struct annotation_info annotation;
> -   memset(&annotation, 0, sizeof(annotation));
> +   struct disasm_info disasm_info = disasm_initialize(devinfo, cfg);
>  
>     foreach_block_and_inst (block, fs_inst, inst, cfg) {
>        struct brw_reg src[3], dst;
> @@ -1666,7 +1665,7 @@ fs_generator::generate_code(const cfg_t *cfg,
> int dispatch_width)
>        }
>  
>        if (unlikely(debug_flag))
> -         annotate(p->devinfo, &annotation, cfg, inst, p-
> >next_insn_offset);
> +         disasm_annotate(&disasm_info, inst, p->next_insn_offset);
>  
>        /* If the instruction writes to more than one register, it
> needs to be
>         * explicitly marked as compressed on Gen <= 5.  On Gen >= 6
> the
> @@ -2129,7 +2128,7 @@ fs_generator::generate_code(const cfg_t *cfg,
> int dispatch_width)
>            */
>           if (!patch_discard_jumps_to_fb_writes()) {
>              if (unlikely(debug_flag)) {
> -               annotation.ann_count--;
> +               disasm_info.use_tail = true;
>              }
>           }
>           break;
> @@ -2189,7 +2188,9 @@ fs_generator::generate_code(const cfg_t *cfg,
> int dispatch_width)
>     }
>  
>     brw_set_uip_jip(p, start_offset);
> -   annotation_finalize(&annotation, p->next_insn_offset);
> +
> +   /* end of program sentinel */
> +   disasm_new_inst_group(&disasm_info, p->next_insn_offset);
>  
>  #ifndef NDEBUG
>     bool validated =
> @@ -2199,11 +2200,10 @@ fs_generator::generate_code(const cfg_t *cfg,
> int dispatch_width)
>        brw_validate_instructions(devinfo, p->store,
>                                  start_offset,
>                                  p->next_insn_offset,
> -                                &annotation);
> +                                &disasm_info);
>  
>     int before_size = p->next_insn_offset - start_offset;
> -   brw_compact_instructions(p, start_offset, annotation.ann_count,
> -                            annotation.ann);
> +   brw_compact_instructions(p, start_offset, &disasm_info);
>     int after_size = p->next_insn_offset - start_offset;
>  
>     if (unlikely(debug_flag)) {
> @@ -2214,9 +2214,8 @@ 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);
> -      ralloc_free(annotation.mem_ctx);
> +      dump_assembly(p->store, &disasm_info);
> +      ralloc_free(disasm_info.mem_ctx);
>     }
>     assert(validated);
>  
> diff --git a/src/intel/compiler/brw_vec4_generator.cpp
> b/src/intel/compiler/brw_vec4_generator.cpp
> index 63831e4ad6..56632cf176 100644
> --- a/src/intel/compiler/brw_vec4_generator.cpp
> +++ b/src/intel/compiler/brw_vec4_generator.cpp
> @@ -1500,8 +1500,7 @@ generate_code(struct brw_codegen *p,
>     const char *stage_abbrev = _mesa_shader_stage_to_abbrev(nir-
> >info.stage);
>     bool debug_flag = INTEL_DEBUG &
>        intel_debug_flag_for_shader_stage(nir->info.stage);
> -   struct annotation_info annotation;
> -   memset(&annotation, 0, sizeof(annotation));
> +   struct disasm_info disasm_info = disasm_initialize(devinfo, cfg);
>     int spill_count = 0, fill_count = 0;
>     int loop_count = 0;
>  
> @@ -1509,7 +1508,7 @@ generate_code(struct brw_codegen *p,
>        struct brw_reg src[3], dst;
>  
>        if (unlikely(debug_flag))
> -         annotate(p->devinfo, &annotation, cfg, inst, p-
> >next_insn_offset);
> +         disasm_annotate(&disasm_info, inst, p->next_insn_offset);
>  
>        for (unsigned int i = 0; i < 3; i++) {
>           src[i] = inst->src[i].as_brw_reg();
> @@ -2175,7 +2174,9 @@ generate_code(struct brw_codegen *p,
>     }
>  
>     brw_set_uip_jip(p, 0);
> -   annotation_finalize(&annotation, p->next_insn_offset);
> +
> +   /* end of program sentinel */
> +   disasm_new_inst_group(&disasm_info, p->next_insn_offset);
>  
>  #ifndef NDEBUG
>     bool validated =
> @@ -2184,10 +2185,10 @@ generate_code(struct brw_codegen *p,
>  #endif
>        brw_validate_instructions(devinfo, p->store,
>                                  0, p->next_insn_offset,
> -                                &annotation);
> +                                &disasm_info);
>  
>     int before_size = p->next_insn_offset;
> -   brw_compact_instructions(p, 0, annotation.ann_count,
> annotation.ann);
> +   brw_compact_instructions(p, 0, &disasm_info);
>     int after_size = p->next_insn_offset;
>  
>     if (unlikely(debug_flag)) {
> @@ -2201,9 +2202,8 @@ generate_code(struct brw_codegen *p,
>                spill_count, fill_count, before_size, after_size,
>                100.0f * (before_size - after_size) / before_size);
>  
> -      dump_assembly(p->store, annotation.ann_count, annotation.ann,
> -                    p->devinfo);
> -      ralloc_free(annotation.mem_ctx);
> +      dump_assembly(p->store, &disasm_info);
> +      ralloc_free(disasm_info.mem_ctx);
>     }
>     assert(validated);
>  
> diff --git a/src/intel/compiler/intel_asm_annotation.c
> b/src/intel/compiler/intel_asm_annotation.c
> index 26ab4b9818..fa37f248d1 100644
> --- a/src/intel/compiler/intel_asm_annotation.c
> +++ b/src/intel/compiler/intel_asm_annotation.c
> @@ -30,51 +30,58 @@
>  __attribute__((weak)) void nir_print_instr(const nir_instr *instr,
> FILE *fp) {}
>  
>  void
> -dump_assembly(void *assembly, int num_annotations, struct annotation
> *annotation,
> -              const struct gen_device_info *devinfo)
> +dump_assembly(void *assembly, struct disasm_info *disasm)
>  {
> +   const struct gen_device_info *devinfo = disasm->devinfo;
>     const char *last_annotation_string = NULL;
>     const void *last_annotation_ir = NULL;
>  
> -   for (int i = 0; i < num_annotations; i++) {
> -      int start_offset = annotation[i].offset;
> -      int end_offset = annotation[i + 1].offset;
> +   foreach_list_typed(struct inst_group, group, link, disasm-
> >group_list) {
> +      struct exec_node *next_node = exec_node_get_next(&group-
> >link);
> +      if (exec_node_is_tail_sentinel(next_node))
> +         break;
>  
> -      if (annotation[i].block_start) {
> -         fprintf(stderr, "   START B%d", annotation[i].block_start-
> >num);
> +      struct inst_group *next =
> +         exec_node_data(struct inst_group, next_node, link);
> +
> +      int start_offset = group->offset;
> +      int end_offset = next->offset;
> +
> +      if (group->block_start) {
> +         fprintf(stderr, "   START B%d", group->block_start->num);
>           foreach_list_typed(struct bblock_link, predecessor_link,
> link,
> -                            &annotation[i].block_start->parents) {
> +                            &group->block_start->parents) {
>              struct bblock_t *predecessor_block = predecessor_link-
> >block;
>              fprintf(stderr, " <-B%d", predecessor_block->num);
>           }
> -         fprintf(stderr, " (%u cycles)\n",
> annotation[i].block_start->cycle_count);
> +         fprintf(stderr, " (%u cycles)\n", group->block_start-
> >cycle_count);
>        }
>  
> -      if (last_annotation_ir != annotation[i].ir) {
> -         last_annotation_ir = annotation[i].ir;
> +      if (last_annotation_ir != group->ir) {
> +         last_annotation_ir = group->ir;
>           if (last_annotation_ir) {
>              fprintf(stderr, "   ");
> -            nir_print_instr(annotation[i].ir, stderr);
> +            nir_print_instr(group->ir, stderr);
>              fprintf(stderr, "\n");
>           }
>        }
>  
> -      if (last_annotation_string != annotation[i].annotation) {
> -         last_annotation_string = annotation[i].annotation;
> +      if (last_annotation_string != group->annotation) {
> +         last_annotation_string = group->annotation;
>           if (last_annotation_string)
>              fprintf(stderr, "   %s\n", last_annotation_string);
>        }
>  
>        brw_disassemble(devinfo, assembly, start_offset, end_offset,
> stderr);
>  
> -      if (annotation[i].error) {
> -         fputs(annotation[i].error, stderr);
> +      if (group->error) {
> +         fputs(group->error, stderr);
>        }
>  
> -      if (annotation[i].block_end) {
> -         fprintf(stderr, "   END B%d", annotation[i].block_end-
> >num);
> +      if (group->block_end) {
> +         fprintf(stderr, "   END B%d", group->block_end->num);
>           foreach_list_typed(struct bblock_link, successor_link,
> link,
> -                            &annotation[i].block_end->children) {
> +                            &group->block_end->children) {
>              struct bblock_t *successor_block = successor_link-
> >block;
>              fprintf(stderr, " ->B%d", successor_block->num);
>           }
> @@ -84,43 +91,55 @@ dump_assembly(void *assembly, int
> num_annotations, struct annotation *annotation
>     fprintf(stderr, "\n");
>  }
>  
> -static bool
> -annotation_array_ensure_space(struct annotation_info *annotation)
> +struct disasm_info
> +disasm_initialize(const struct gen_device_info *devinfo,
> +                  const struct cfg_t *cfg)
>  {
> -   if (annotation->ann_size <= annotation->ann_count) {
> -      int old_size = annotation->ann_size;
> -      annotation->ann_size = MAX2(1024, annotation->ann_size * 2);
> -      annotation->ann = reralloc(annotation->mem_ctx, annotation-
> >ann,
> -                                 struct annotation, annotation-
> >ann_size);
> -      if (!annotation->ann)
> -         return false;
> -
> -      memset(annotation->ann + old_size, 0,
> -             (annotation->ann_size - old_size) * sizeof(struct
> annotation));
> -   }
> -
> -   return true;
> +   struct disasm_info disasm = {
> +      .cur_block = 0,
> +      .devinfo = devinfo,
> +      .cfg = cfg,
> +   };
> +   disasm.mem_ctx = ralloc_context(NULL);
> +   disasm.group_list = ralloc(disasm.mem_ctx, struct exec_list);
> +   exec_list_make_empty(disasm.group_list);
> +
> +   return disasm;
>  }
>  
> -void annotate(const struct gen_device_info *devinfo,
> -              struct annotation_info *annotation, const struct cfg_t
> *cfg,
> -              struct backend_instruction *inst, unsigned offset)
> +struct inst_group *
> +disasm_new_inst_group(struct disasm_info *disasm, unsigned
> next_inst_offset)
>  {
> -   if (annotation->mem_ctx == NULL)
> -      annotation->mem_ctx = ralloc_context(NULL);
> +   struct inst_group *tail =
> +      rzalloc(disasm->mem_ctx, struct inst_group);
> +   tail->offset = next_inst_offset;
> +   exec_list_push_tail(disasm->group_list, &tail->link);
> +   return tail;
> +}
>  
> -   if (!annotation_array_ensure_space(annotation))
> -      return;
> +void
> +disasm_annotate(struct disasm_info *disasm,
> +                struct backend_instruction *inst, unsigned offset)
> +{
> +   const struct gen_device_info *devinfo = disasm->devinfo;
> +   const struct cfg_t *cfg = disasm->cfg;
> +
> +   struct inst_group *group;
> +   if (!disasm->use_tail) {
> +      group = disasm_new_inst_group(disasm, offset);
> +      disasm->use_tail = false;
> +   } else {
> +      group = exec_node_data(struct inst_group,
> +                             exec_list_get_tail_raw(disasm-
> >group_list), link);
> +   }
>  
> -   struct annotation *ann = &annotation->ann[annotation-
> >ann_count++];
> -   ann->offset = offset;
>     if ((INTEL_DEBUG & DEBUG_ANNOTATION) != 0) {
> -      ann->ir = inst->ir;
> -      ann->annotation = inst->annotation;
> +      group->ir = inst->ir;
> +      group->annotation = inst->annotation;
>     }
>  
> -   if (bblock_start(cfg->blocks[annotation->cur_block]) == inst) {
> -      ann->block_start = cfg->blocks[annotation->cur_block];
> +   if (bblock_start(cfg->blocks[disasm->cur_block]) == inst) {
> +      group->block_start = cfg->blocks[disasm->cur_block];
>     }
>  
>     /* There is no hardware DO instruction on Gen6+, so since DO
> always
> @@ -132,66 +151,48 @@ void annotate(const struct gen_device_info
> *devinfo,
>      * a corresponding hardware instruction to disassemble.
>      */
>     if (devinfo->gen >= 6 && inst->opcode == BRW_OPCODE_DO) {
> -      annotation->ann_count--;
> +      disasm->use_tail = true;
>     }
>  
> -   if (bblock_end(cfg->blocks[annotation->cur_block]) == inst) {
> -      ann->block_end = cfg->blocks[annotation->cur_block];
> -      annotation->cur_block++;
> -   }
> -}
> -
> -void
> -annotation_finalize(struct annotation_info *annotation,
> -                    unsigned next_inst_offset)
> -{
> -   if (!annotation->ann_count)
> -      return;
> -
> -   if (annotation->ann_count == annotation->ann_size) {
> -      annotation->ann = reralloc(annotation->mem_ctx, annotation-
> >ann,
> -                                 struct annotation, annotation-
> >ann_size + 1);
> +   if (bblock_end(cfg->blocks[disasm->cur_block]) == inst) {
> +      group->block_end = cfg->blocks[disasm->cur_block];
> +      disasm->cur_block++;
>     }
> -   annotation->ann[annotation->ann_count].offset = next_inst_offset;
>  }
>  
>  void
> -annotation_insert_error(struct annotation_info *annotation, unsigned
> offset,
> -                        const char *error)
> +disasm_insert_error(struct disasm_info *disasm, unsigned offset,
> +                    const char *error)
>  {
> -   if (!annotation->ann_count)
> -      return;
> -
> -   /* We may have to split an annotation, so ensure we have enough
> space
> -    * allocated for that case up front.
> -    */
> -   if (!annotation_array_ensure_space(annotation))
> -      return;
> -
> -   assume(annotation->ann_count > 0);
> +   foreach_list_typed(struct inst_group, cur, link, disasm-
> >group_list) {
> +      struct exec_node *next_node = exec_node_get_next(&cur->link);
> +      if (exec_node_is_tail_sentinel(next_node))
> +         break;
>  
> -   for (int i = 0; i < annotation->ann_count; i++) {
> -      struct annotation *cur = &annotation->ann[i];
> -      struct annotation *next = &annotation->ann[i + 1];
> +      struct inst_group *next =
> +         exec_node_data(struct inst_group, next_node, link);
>  
>        if (next->offset <= offset)
>           continue;
>  
>        if (offset + sizeof(brw_inst) != next->offset) {
> -         memmove(next, cur,
> -                 (annotation->ann_count - i + 2) * sizeof(struct
> annotation));
> +         struct inst_group *new = ralloc(disasm->mem_ctx, struct
> inst_group);
> +         memcpy(new, cur, sizeof(struct inst_group));
> +
>           cur->error = NULL;
>           cur->error_length = 0;
>           cur->block_end = NULL;
> -         next->offset = offset + sizeof(brw_inst);
> -         next->block_start = NULL;
> -         annotation->ann_count++;
> +
> +         new->offset = offset + sizeof(brw_inst);
> +         new->block_start = NULL;
> +
> +         exec_node_insert_after(&cur->link, &new->link);
>        }
>  
>        if (cur->error)
>           ralloc_strcat(&cur->error, error);
>        else
> -         cur->error = ralloc_strdup(annotation->mem_ctx, error);
> +         cur->error = ralloc_strdup(disasm->mem_ctx, error);
>        return;
>     }
>  }
> diff --git a/src/intel/compiler/intel_asm_annotation.h
> b/src/intel/compiler/intel_asm_annotation.h
> index 2d905b10a9..515c799ca1 100644
> --- a/src/intel/compiler/intel_asm_annotation.h
> +++ b/src/intel/compiler/intel_asm_annotation.h
> @@ -24,14 +24,15 @@
>  #ifndef _INTEL_ASM_ANNOTATION_H
>  #define _INTEL_ASM_ANNOTATION_H
>  
> +#include "brw_cfg.h"
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
>  
> -struct backend_instruction;
> -struct cfg_t;
> +struct inst_group {
> +   struct exec_node link;
>  
> -struct annotation {
>     int offset;
>  
>     size_t error_length;
> @@ -48,30 +49,35 @@ struct annotation {
>     const char *annotation;
>  };
>  
> -struct annotation_info {
> +struct disasm_info {
> +   struct exec_list *group_list;
>     void *mem_ctx;
> -   struct annotation *ann;
> -   int ann_count;
> -   int ann_size;
> +
> +   const struct gen_device_info *devinfo;
> +   const struct cfg_t *cfg;
>  
>     /** Block index in the cfg. */
>     int cur_block;
> +   bool use_tail;
>  };
>  
>  void
> -dump_assembly(void *assembly, int num_annotations, struct annotation
> *annotation,
> -              const struct gen_device_info *devinfo);
> +dump_assembly(void *assembly, struct disasm_info *disasm);
> +
> +struct disasm_info
> +disasm_initialize(const struct gen_device_info *devinfo,
> +                  const struct cfg_t *cfg);
> +
> +struct inst_group *
> +disasm_new_inst_group(struct disasm_info *disasm, unsigned offset);
>  
>  void
> -annotate(const struct gen_device_info *devinfo,
> -         struct annotation_info *annotation, const struct cfg_t
> *cfg,
> -         struct backend_instruction *inst, unsigned offset);
> -void
> -annotation_finalize(struct annotation_info *annotation, unsigned
> offset);
> +disasm_annotate(struct disasm_info *disasm,
> +                struct backend_instruction *inst, unsigned offset);
>  
>  void
> -annotation_insert_error(struct annotation_info *annotation, unsigned
> offset,
> -                        const char *error);
> +disasm_insert_error(struct disasm_info *disasm, unsigned offset,
> +                    const char *error);
>  
>  #ifdef __cplusplus
>  } /* extern "C" */
> diff --git a/src/intel/compiler/test_eu_validate.cpp
> b/src/intel/compiler/test_eu_validate.cpp
> index e8b175beea..b147b6b227 100644
> --- a/src/intel/compiler/test_eu_validate.cpp
> +++ b/src/intel/compiler/test_eu_validate.cpp
> @@ -113,25 +113,20 @@ static bool
>  validate(struct brw_codegen *p)
>  {
>     const bool print = getenv("TEST_DEBUG");
> -   struct annotation_info annotation;
> -   memset(&annotation, 0, sizeof(annotation));
> +   struct disasm_info disasm = disasm_initialize(p->devinfo, NULL);
>  
>     if (print) {
> -      annotation.mem_ctx = ralloc_context(NULL);
> -      annotation.ann_count = 1;
> -      annotation.ann_size = 2;
> -      annotation.ann = rzalloc_array(annotation.mem_ctx, struct
> annotation,
> -                                     annotation.ann_size);
> -      annotation.ann[annotation.ann_count].offset = p-
> >next_insn_offset;
> +      disasm_new_inst_group(&disasm, 0);
> +      disasm_new_inst_group(&disasm, p->next_insn_offset);
>     }
>  
>     bool ret = brw_validate_instructions(p->devinfo, p->store, 0,
> -                                        p->next_insn_offset,
> &annotation);
> +                                        p->next_insn_offset,
> &disasm);
>  
>     if (print) {
> -      dump_assembly(p->store, annotation.ann_count, annotation.ann,
> p->devinfo);
> -      ralloc_free(annotation.mem_ctx);
> +      dump_assembly(p->store, &disasm);
>     }
> +   ralloc_free(disasm.mem_ctx);
>  
>     return ret;
>  }
> diff --git a/src/intel/tools/disasm.c b/src/intel/tools/disasm.c
> index 251acd313d..153da6b6c6 100644
> --- a/src/intel/tools/disasm.c
> +++ b/src/intel/tools/disasm.c
> @@ -76,34 +76,34 @@ gen_disasm_disassemble(struct gen_disasm *disasm,
> void *assembly,
>     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
> +   /* Make a dummy disasm 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;
> +   struct disasm_info disasm_info = disasm_initialize(devinfo,
> NULL);
> +   disasm_new_inst_group(&disasm_info, start);
> +   disasm_new_inst_group(&disasm_info, end);
> +
> +   brw_validate_instructions(devinfo, assembly, start, end,
> &disasm_info);
> +
> +   foreach_list_typed(struct inst_group, group, link,
> disasm_info.group_list) {
> +      struct exec_node *next_node = exec_node_get_next(&group-
> >link);
> +      if (exec_node_is_tail_sentinel(next_node))
> +         break;
> +
> +      struct inst_group *next =
> +         exec_node_data(struct inst_group, next_node, link);
> +
> +      int start_offset = group->offset;
> +      int end_offset = next->offset;
>  
>        brw_disassemble(devinfo, assembly, start_offset, end_offset,
> out);
>  
> -      if (annotation[i].error) {
> -         fputs(annotation[i].error, out);
> +      if (group->error) {
> +         fputs(group->error, out);
>        }
>     }
>  
> -   ralloc_free(annotation_info.mem_ctx);
> +   ralloc_free(disasm_info.mem_ctx);
>  }
>  
>  struct gen_disasm *
> diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
> b/src/mesa/drivers/dri/i965/brw_ff_gs.c
> index f535537d75..174418a474 100644
> --- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
> @@ -127,7 +127,7 @@ brw_codegen_ff_gs_prog(struct brw_context *brw,
>        }
>     }
>  
> -   brw_compact_instructions(&c.func, 0, 0, NULL);
> +   brw_compact_instructions(&c.func, 0, NULL);
>  
>     /* get the program
>      */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171117/e558b472/attachment-0001.sig>


More information about the mesa-dev mailing list