i965/vs: Make virtual grf live intervals actually cover their used range.

This is the same change as the previous commit to the FS.  A very few VSes
are regressed by 1 or 2 instructions, which look recoverable with a bit
more dead code elimination.

Reviewed-by: Ian Romanick <[email protected]>

4 files changed, 31 insertions, 63 deletions

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index c6ca4535003..c5aa338a72a 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -305,9 +305,12 @@ vec4_visitor::dead_code_eliminate()
    foreach_list_safe(node, &this->instructions) {
       vec4_instruction *inst = (vec4_instruction *)node;
 
-      if (inst->dst.file == GRF && this->virtual_grf_use[inst->dst.reg] <= pc) {
-	 inst->remove();
-	 progress = true;
+      if (inst->dst.file == GRF) {
+         assert(this->virtual_grf_end[inst->dst.reg] >= pc);
+         if (this->virtual_grf_end[inst->dst.reg] == pc) {
+            inst->remove();
+            progress = true;
+         }
       }
 
       pc++;
@@ -832,7 +835,7 @@ vec4_visitor::opt_register_coalesce()
       /* Can't coalesce this GRF if someone else was going to
        * read it later.
        */
-      if (this->virtual_grf_use[inst->src[0].reg] > ip)
+      if (this->virtual_grf_end[inst->src[0].reg] > ip)
 	 continue;
 
       /* We need to check interference with the final destination between this
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
index 7614ac58652..e6e59bc9af9 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -238,8 +238,8 @@ public:
    int virtual_grf_array_size;
    int first_non_payload_grf;
    unsigned int max_grf;
-   int *virtual_grf_def;
-   int *virtual_grf_use;
+   int *virtual_grf_start;
+   int *virtual_grf_end;
    dst_reg userplane[MAX_CLIP_PLANES];
 
    /**
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
index f34111c3317..db3787bfd91 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
@@ -183,8 +183,8 @@ vec4_live_variables::~vec4_live_variables()
  * We could expose per-channel live intervals to the consumer based on the
  * information we computed in vec4_live_variables, except that our only
  * current user is virtual_grf_interferes().  So we instead union the
- * per-channel ranges into a per-vgrf range for virtual_grf_def[] and
- * virtual_grf_use[].
+ * per-channel ranges into a per-vgrf range for virtual_grf_start[] and
+ * virtual_grf_end[].
  *
  * We could potentially have virtual_grf_interferes() do the test per-channel,
  * which would let some interesting register allocation occur (particularly on
@@ -200,16 +200,16 @@ vec4_visitor::calculate_live_intervals()
    if (this->live_intervals_valid)
       return;
 
-   int *def = ralloc_array(mem_ctx, int, this->virtual_grf_count);
-   int *use = ralloc_array(mem_ctx, int, this->virtual_grf_count);
-   ralloc_free(this->virtual_grf_def);
-   ralloc_free(this->virtual_grf_use);
-   this->virtual_grf_def = def;
-   this->virtual_grf_use = use;
+   int *start = ralloc_array(mem_ctx, int, this->virtual_grf_count);
+   int *end = ralloc_array(mem_ctx, int, this->virtual_grf_count);
+   ralloc_free(this->virtual_grf_start);
+   ralloc_free(this->virtual_grf_end);
+   this->virtual_grf_start = start;
+   this->virtual_grf_end = end;
 
    for (int i = 0; i < this->virtual_grf_count; i++) {
-      def[i] = MAX_INSTRUCTION;
-      use[i] = -1;
+      start[i] = MAX_INSTRUCTION;
+      end[i] = -1;
    }
 
    /* Start by setting up the intervals with no knowledge of control
@@ -223,14 +223,16 @@ vec4_visitor::calculate_live_intervals()
 	 if (inst->src[i].file == GRF) {
 	    int reg = inst->src[i].reg;
 
-	    use[reg] = ip;
+            start[reg] = MIN2(start[reg], ip);
+            end[reg] = ip;
 	 }
       }
 
       if (inst->dst.file == GRF) {
          int reg = inst->dst.reg;
 
-         def[reg] = MIN2(def[reg], ip);
+         start[reg] = MIN2(start[reg], ip);
+         end[reg] = ip;
       }
 
       ip++;
@@ -247,60 +249,23 @@ vec4_visitor::calculate_live_intervals()
    for (int b = 0; b < cfg.num_blocks; b++) {
       for (int i = 0; i < livevars.num_vars; i++) {
 	 if (livevars.bd[b].livein[i]) {
-	    def[i / 4] = MIN2(def[i / 4], cfg.blocks[b]->start_ip);
-	    use[i / 4] = MAX2(use[i / 4], cfg.blocks[b]->start_ip);
+	    start[i / 4] = MIN2(start[i / 4], cfg.blocks[b]->start_ip);
+	    end[i / 4] = MAX2(end[i / 4], cfg.blocks[b]->start_ip);
 	 }
 
 	 if (livevars.bd[b].liveout[i]) {
-	    def[i / 4] = MIN2(def[i / 4], cfg.blocks[b]->end_ip);
-	    use[i / 4] = MAX2(use[i / 4], cfg.blocks[b]->end_ip);
+	    start[i / 4] = MIN2(start[i / 4], cfg.blocks[b]->end_ip);
+	    end[i / 4] = MAX2(end[i / 4], cfg.blocks[b]->end_ip);
 	 }
       }
    }
 
    this->live_intervals_valid = true;
-
-   /* Note in the non-control-flow code above, that we only take def[] as the
-    * first store, and use[] as the last use.  We use this in dead code
-    * elimination, to determine when a store never gets used.  However, we
-    * also use these arrays to answer the virtual_grf_interferes() question
-    * (live interval analysis), which is used for register coalescing and
-    * register allocation.
-    *
-    * So, there's a conflict over what the array should mean: if use[]
-    * considers a def after the last use, then the dead code elimination pass
-    * never does anything (and it's an important pass!).  But if we don't
-    * include dead code, then virtual_grf_interferes() lies and we'll do
-    * horrible things like coalesce the register that is dead-code-written
-    * into another register that was live across the dead write (causing the
-    * use of the second register to take the dead write's source value instead
-    * of the coalesced MOV's source value).
-    *
-    * To resolve the conflict, immediately after calculating live intervals,
-    * detect dead code, nuke it, and if we changed anything, calculate again
-    * before returning to the caller.  Now we happen to produce def[] and
-    * use[] arrays that will work for virtual_grf_interferes().
-    */
-   if (dead_code_eliminate())
-      calculate_live_intervals();
 }
 
 bool
 vec4_visitor::virtual_grf_interferes(int a, int b)
 {
-   int a_def = this->virtual_grf_def[a], a_use = this->virtual_grf_use[a];
-   int b_def = this->virtual_grf_def[b], b_use = this->virtual_grf_use[b];
-
-   /* If there's dead code (def but not use), it would break our test
-    * unless we consider it used.
-    */
-   if ((a_use == -1 && a_def != MAX_INSTRUCTION) ||
-       (b_use == -1 && b_def != MAX_INSTRUCTION)) {
-      return true;
-   }
-
-   int start = MAX2(a_def, b_def);
-   int end = MIN2(a_use, b_use);
-
-   return start < end;
+   return !(virtual_grf_end[a] <= virtual_grf_start[b] ||
+            virtual_grf_end[b] <= virtual_grf_start[a]);
 }
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index cda425efde8..e5c1cece337 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -3173,8 +3173,8 @@ vec4_visitor::vec4_visitor(struct brw_context *brw,
 				       hash_table_pointer_hash,
 				       hash_table_pointer_compare);
 
-   this->virtual_grf_def = NULL;
-   this->virtual_grf_use = NULL;
+   this->virtual_grf_start = NULL;
+   this->virtual_grf_end = NULL;
    this->virtual_grf_sizes = NULL;
    this->virtual_grf_count = 0;
    this->virtual_grf_reg_map = NULL;