[Mesa-dev] [PATCH] i965: Invalidate virtual register information after register allocation.

Kenneth Graunke kenneth at whitecape.org
Tue Jun 9 17:23:08 PDT 2015


Connor and I have both hit bugs where code (such as dump_instructions())
attempts to use live intervals or VGRF information, and wasted a bunch
of time trying to debug that.

By freeing the arrays and zeroing the counts, we make any array access
out of bounds, which results in an obvious problem that's easy to
uncover with Valgrind, rather than subtle bugs.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Cc: Connor Abbott <cwabbott0 at gmail.com>
Cc: Matt Turner <mattst88 at gmail.com>
---
 src/mesa/drivers/dri/i965/brw_fs.cpp         |  6 ++++++
 src/mesa/drivers/dri/i965/brw_ir_allocator.h | 23 +++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 7789ca7..d663a1e 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -3827,6 +3827,12 @@ fs_visitor::allocate_registers()
 
    if (last_scratch > 0)
       prog_data->total_scratch = brw_get_scratch_size(last_scratch);
+
+   /* Invalidate any information about virtual registers.  They no longer
+    * exist; any attempt to access that information is a bug.
+    */
+   invalidate_live_intervals();
+   alloc.invalidate();
 }
 
 bool
diff --git a/src/mesa/drivers/dri/i965/brw_ir_allocator.h b/src/mesa/drivers/dri/i965/brw_ir_allocator.h
index b1237ed..e67947a 100644
--- a/src/mesa/drivers/dri/i965/brw_ir_allocator.h
+++ b/src/mesa/drivers/dri/i965/brw_ir_allocator.h
@@ -40,8 +40,7 @@ namespace brw {
 
       ~simple_allocator()
       {
-         free(offsets);
-         free(sizes);
+         invalidate();
       }
 
       unsigned
@@ -61,6 +60,26 @@ namespace brw {
       }
 
       /**
+       * Throw away all information about virtual registers.
+       *
+       * After register allocation, we no longer have virtual registers, so
+       * any attempts to access information about them are likely bugs.  This
+       * method provides a way to throw away all the information so attempts
+       * to access it result in obvious crashes rather than subtle bugs.
+       */
+      void
+      invalidate()
+      {
+         free(offsets);
+         offsets = NULL;
+         free(sizes);
+         sizes = NULL;
+         count = 0;
+         total_size = 0;
+         capacity = 0;
+      }
+
+      /**
        * Array of sizes for each allocation.  The allocation unit is up to the
        * back-end, but it's expected to be one scalar value in the FS back-end
        * and one vec4 in the VEC4 back-end.
-- 
2.4.2



More information about the mesa-dev mailing list