Mesa (main): intel/eu: Handle compaction when inserting validation errors

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 82ee30e55861b237b0a0e23cd31133cb8778fd7b
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=82ee30e55861b237b0a0e23cd31133cb8778fd7b

Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Tue Jul 19 00:27:29 2022 -0700

intel/eu: Handle compaction when inserting validation errors

When the EU validator encountered an error, it would add an annotation
to the disassembly.  Unfortunately, the code to insert an error assumed
that the next instruction would start at (offset + sizeof(brw_inst)),
which is not true if the instruction with an error is compacted.

This could lead to cascading disassembly errors, where we started trying
to decode the next instruction at the wrong offset, and getting lots of
scary looking output:

   ERROR: Register Regioning patterns where [...]
   (-f0.1.any16h) illegal(*** invalid execution size value 6 )      { align1 $7.src atomic };
   (+f0.1.any16h) illegal.sat(*** invalid execution size value 6 )  { align1 $9.src AccWrEnable };
   illegal(*** invalid execution size value 6 )                     { align1 $11.src };
   (+f0.1) illegal.sat(*** invalid execution size value 6 )         { align1 F at 2 AccWrEnable };
   (+f0.1) illegal.sat(*** invalid execution size value 6 )         { align1 F at 2 AccWrEnable };
   (+f0.1) illegal.sat(*** invalid execution size value 6 )         { align1 $15.src AccWrEnable };
   illegal(*** invalid execution size value 6 )                     { align1 $15.src };
   (+f0.1) illegal.sat.g.f0.1(*** invalid execution size value 6 )  { align1 $13.src AccWrEnable };

Only the first instruction was actually wrong - the rest are just a
result of starting the disassembler at the wrong offset.  Trash ensues!

To fix this, just pass the instruction size in a few layers so we can
record the next offset properly.

Cc: mesa-stable
Reviewed-by: Francisco Jerez <currojerez at riseup.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17624>

---

 src/intel/compiler/brw_disasm_info.c   | 6 +++---
 src/intel/compiler/brw_disasm_info.h   | 2 +-
 src/intel/compiler/brw_eu.h            | 1 +
 src/intel/compiler/brw_eu_validate.c   | 6 ++++--
 src/intel/compiler/test_eu_compact.cpp | 2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/intel/compiler/brw_disasm_info.c b/src/intel/compiler/brw_disasm_info.c
index 779e25f9db6..9ee365f0e60 100644
--- a/src/intel/compiler/brw_disasm_info.c
+++ b/src/intel/compiler/brw_disasm_info.c
@@ -170,7 +170,7 @@ disasm_annotate(struct disasm_info *disasm,
 
 void
 disasm_insert_error(struct disasm_info *disasm, unsigned offset,
-                    const char *error)
+                    unsigned inst_size, const char *error)
 {
    foreach_list_typed(struct inst_group, cur, link, &disasm->group_list) {
       struct exec_node *next_node = exec_node_get_next(&cur->link);
@@ -183,7 +183,7 @@ disasm_insert_error(struct disasm_info *disasm, unsigned offset,
       if (next->offset <= offset)
          continue;
 
-      if (offset + sizeof(brw_inst) != next->offset) {
+      if (offset + inst_size != next->offset) {
          struct inst_group *new = ralloc(disasm, struct inst_group);
          memcpy(new, cur, sizeof(struct inst_group));
 
@@ -191,7 +191,7 @@ disasm_insert_error(struct disasm_info *disasm, unsigned offset,
          cur->error_length = 0;
          cur->block_end = NULL;
 
-         new->offset = offset + sizeof(brw_inst);
+         new->offset = offset + inst_size;
          new->block_start = NULL;
 
          exec_node_insert_after(&cur->link, &new->link);
diff --git a/src/intel/compiler/brw_disasm_info.h b/src/intel/compiler/brw_disasm_info.h
index c45b98a0209..937180b7e2e 100644
--- a/src/intel/compiler/brw_disasm_info.h
+++ b/src/intel/compiler/brw_disasm_info.h
@@ -81,7 +81,7 @@ disasm_annotate(struct disasm_info *disasm,
 
 void
 disasm_insert_error(struct disasm_info *disasm, unsigned offset,
-                    const char *error);
+                    unsigned inst_size, const char *error);
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/src/intel/compiler/brw_eu.h b/src/intel/compiler/brw_eu.h
index 72c0bc8682d..e27b5069dd6 100644
--- a/src/intel/compiler/brw_eu.h
+++ b/src/intel/compiler/brw_eu.h
@@ -1883,6 +1883,7 @@ void brw_debug_compact_uncompact(const struct brw_isa_info *isa,
 /* brw_eu_validate.c */
 bool brw_validate_instruction(const struct brw_isa_info *isa,
                               const brw_inst *inst, int offset,
+                              unsigned inst_size,
                               struct disasm_info *disasm);
 bool brw_validate_instructions(const struct brw_isa_info *isa,
                                const void *assembly, int start_offset, int end_offset,
diff --git a/src/intel/compiler/brw_eu_validate.c b/src/intel/compiler/brw_eu_validate.c
index 4c26889187e..0db30a46771 100644
--- a/src/intel/compiler/brw_eu_validate.c
+++ b/src/intel/compiler/brw_eu_validate.c
@@ -2270,6 +2270,7 @@ send_descriptor_restrictions(const struct brw_isa_info *isa,
 bool
 brw_validate_instruction(const struct brw_isa_info *isa,
                          const brw_inst *inst, int offset,
+                         unsigned inst_size,
                          struct disasm_info *disasm)
 {
    struct string error_msg = { .str = NULL, .len = 0 };
@@ -2295,7 +2296,7 @@ brw_validate_instruction(const struct brw_isa_info *isa,
    }
 
    if (error_msg.str && disasm) {
-      disasm_insert_error(disasm, offset, error_msg.str);
+      disasm_insert_error(disasm, offset, inst_size, error_msg.str);
    }
    free(error_msg.str);
 
@@ -2323,7 +2324,8 @@ brw_validate_instructions(const struct brw_isa_info *isa,
          inst = &uncompacted;
       }
 
-      bool v = brw_validate_instruction(isa, inst, src_offset, disasm);
+      bool v = brw_validate_instruction(isa, inst, src_offset,
+                                        inst_size, disasm);
       valid = valid && v;
 
       src_offset += inst_size;
diff --git a/src/intel/compiler/test_eu_compact.cpp b/src/intel/compiler/test_eu_compact.cpp
index fde374739dc..59ad71f3c7f 100644
--- a/src/intel/compiler/test_eu_compact.cpp
+++ b/src/intel/compiler/test_eu_compact.cpp
@@ -189,7 +189,7 @@ test_fuzz_compact_instruction(struct brw_codegen *p, brw_inst src)
 
          clear_pad_bits(p->isa, &instr);
 
-         if (!brw_validate_instruction(p->isa, &instr, 0, NULL))
+         if (!brw_validate_instruction(p->isa, &instr, 0, sizeof(brw_inst), NULL))
             continue;
 
 	 if (!test_compact_instruction(p, instr)) {