intel/brw: Simplify fs_inst annotation

When INTEL_DEBUG=ann is also set, the disassembler would annotate the
output with either a string or the string verison of a NIR instruction.
This was done by keeping two pointers (but only using one at a time).

Change the code to print the instruction into a string instead of
keeping it pointer around (peg the string to the shader).  That way,
only one pointer is needed for annotations.  Because that serialization
is not free, only do that when the environment variable is set.

Since we are here, move the annotation string field to the end, moving
it to the least commonly used cacheline.  Further packing might allow
the entire fs_inst to fit in two cachelines.

For release builds, don't even add the debug annotation to the struct.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/30822>

src/intel/compiler/brw_disasm_info.cpp

@@ -34,7 +34,6 @@ dump_assembly(void *assembly, int start_offset, int end_offset,
 {
    const struct brw_isa_info *isa = disasm->isa;
    const char *last_annotation_string = NULL;
-   const void *last_annotation_ir = NULL;
 
    void *mem_ctx = ralloc_context(NULL);
    const struct brw_label *root_label =
@@ -64,15 +63,6 @@ dump_assembly(void *assembly, int start_offset, int end_offset,
          fprintf(stderr, "\n");
       }
 
-      if (last_annotation_ir != group->ir) {
-         last_annotation_ir = group->ir;
-         if (last_annotation_ir) {
-            fprintf(stderr, "   ");
-            nir_print_instr((nir_instr *)group->ir, stderr);
-            fprintf(stderr, "\n");
-         }
-      }
-
       if (last_annotation_string != group->annotation) {
          last_annotation_string = group->annotation;
          if (last_annotation_string)
@@ -139,10 +129,11 @@ disasm_annotate(struct disasm_info *disasm,
                              exec_list_get_tail_raw(&disasm->group_list), link);
    }
 
+#ifndef NDEBUG
    if (INTEL_DEBUG(DEBUG_ANNOTATION)) {
-      group->ir = inst->ir;
       group->annotation = inst->annotation;
    }
+#endif
 
    if (bblock_start(cfg->blocks[disasm->cur_block]) == inst) {
       group->block_start = cfg->blocks[disasm->cur_block];

src/intel/compiler/brw_disasm_info.h

@@ -48,8 +48,7 @@ struct inst_group {
    struct bblock_t *block_start;
    struct bblock_t *block_end;
 
-   /* Annotation for the generated IR.  One of the two can be set. */
-   const void *ir;
+   /* Annotation for the generated IR. */
    const char *annotation;
 };
 

src/intel/compiler/brw_fs_builder.h

@@ -63,8 +63,11 @@ namespace brw {
          _group(inst->group),
          force_writemask_all(inst->force_writemask_all)
       {
+#ifndef NDEBUG
          annotation.str = inst->annotation;
-         annotation.ir = inst->ir;
+#else
+         annotation.str = NULL;
+#endif
       }
 
       /**
@@ -152,11 +155,10 @@ namespace brw {
        * Construct a builder with the given debug annotation info.
        */
       fs_builder
-      annotate(const char *str, const void *ir = NULL) const
+      annotate(const char *str) const
       {
          fs_builder bld = *this;
          bld.annotation.str = str;
-         bld.annotation.ir = ir;
          return bld;
       }
 
@@ -334,8 +336,9 @@ namespace brw {
 
          inst->group = _group;
          inst->force_writemask_all = force_writemask_all;
+#ifndef NDEBUG
          inst->annotation = annotation.str;
-         inst->ir = annotation.ir;
+#endif
 
          if (block)
             static_cast<fs_inst *>(cursor)->insert_before(block, inst);
@@ -966,7 +969,6 @@ namespace brw {
       /** Debug annotation info. */
       struct {
          const char *str;
-         const void *ir;
       } annotation;
    };
 }

src/intel/compiler/brw_fs_lower.cpp

@@ -796,7 +796,7 @@ brw_fs_lower_load_subgroup_invocation(fs_visitor &s)
          continue;
 
       const fs_builder abld =
-         fs_builder(&s, block, inst).annotate("SubgroupInvocation", NULL);
+         fs_builder(&s, block, inst).annotate("SubgroupInvocation");
       const fs_builder ubld8 = abld.group(8, 0).exec_all();
       ubld8.UNDEF(inst->dst);
 

src/intel/compiler/brw_fs_nir.cpp

@@ -28,6 +28,7 @@
 #include "nir.h"
 #include "nir_intrinsics.h"
 #include "nir_search_helpers.h"
+#include "dev/intel_debug.h"
 #include "util/u_math.h"
 #include "util/bitscan.h"
 
@@ -59,6 +60,8 @@ struct nir_to_brw_state {
    struct brw_fs_bind_info *ssa_bind_infos;
    brw_reg *uniform_values;
    brw_reg *system_values;
+
+   bool annotate;
 };
 
 static brw_reg get_nir_src(nir_to_brw_state &ntb, const nir_src &src);
@@ -301,7 +304,7 @@ emit_system_values_block(nir_to_brw_state &ntb, nir_block *block)
          reg = &ntb.system_values[SYSTEM_VALUE_HELPER_INVOCATION];
          if (reg->file == BAD_FILE) {
             const fs_builder abld =
-               ntb.bld.annotate("gl_HelperInvocation", NULL);
+               ntb.bld.annotate("gl_HelperInvocation");
 
             /* On Gfx6+ (gl_HelperInvocation is only exposed on Gfx7+) the
              * pixel mask is in g1.7 of the thread payload.
@@ -2464,7 +2467,7 @@ set_gs_stream_control_data_bits(nir_to_brw_state &ntb, const brw_reg &vertex_cou
    if (stream_id == 0)
       return;
 
-   const fs_builder abld = ntb.bld.annotate("set stream control data bits", NULL);
+   const fs_builder abld = ntb.bld.annotate("set stream control data bits");
 
    /* reg::sid = stream_id */
    brw_reg sid = abld.MOV(brw_imm_ud(stream_id));
@@ -8725,7 +8728,12 @@ shuffle_from_32bit_read(const fs_builder &bld,
 static void
 fs_nir_emit_instr(nir_to_brw_state &ntb, nir_instr *instr)
 {
-   ntb.bld = ntb.bld.annotate(NULL, instr);
+#ifndef NDEBUG
+   if (unlikely(ntb.annotate)) {
+      /* Use shader mem_ctx since annotations outlive the NIR conversion. */
+      ntb.bld = ntb.bld.annotate(nir_instr_as_str(instr, ntb.s.mem_ctx));
+   }
+#endif
 
    switch (instr->type) {
    case nir_instr_type_alu:
@@ -8913,6 +8921,9 @@ nir_to_brw(fs_visitor *s)
       .bld     = fs_builder(s).at_end(),
    };
 
+   if (INTEL_DEBUG(DEBUG_ANNOTATION))
+      ntb.annotate = true;
+
    if (ENABLE_FS_TEST_DISPATCH_PACKING)
       brw_fs_test_dispatch_packing(ntb.bld);
 

src/intel/compiler/brw_ir_fs.h

@@ -122,13 +122,6 @@ public:
     */
    bool has_sampler_residency() const;
 
-   /** @{
-    * Annotation for the generated IR.  One of the two can be set.
-    */
-   const void *ir;
-   const char *annotation;
-   /** @} */
-
    uint8_t sources; /**< Number of brw_reg sources. */
 
    /**
@@ -225,6 +218,14 @@ public:
    brw_reg dst;
    brw_reg *src;
    brw_reg builtin_src[4];
+
+#ifndef NDEBUG
+   /** @{
+    * Annotation for the generated IR.
+    */
+   const char *annotation;
+   /** @} */
+#endif
 };
 
 /**