Mesa (master): i965/vs: Make virtual grf live intervals actually cover their used range.
Eric Anholt
anholt at kemper.freedesktop.org
Thu May 9 21:38:19 UTC 2013
Module: Mesa
Branch: master
Commit: 0f3068a58bdbceb2cb93e3848b0e2145629cdf43
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=0f3068a58bdbceb2cb93e3848b0e2145629cdf43
Author: Eric Anholt <eric at anholt.net>
Date: Tue Apr 30 15:15:21 2013 -0700
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 <ian.d.romanick at intel.com>
---
src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 ++-
src/mesa/drivers/dri/i965/brw_vec4.h | 4 +-
.../drivers/dri/i965/brw_vec4_live_variables.cpp | 75 +++++--------------
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 4 +-
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 c6ca453..c5aa338 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 7614ac5..e6e59bc 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 f34111c..db3787b 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 cda425e..e5c1cec 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;
More information about the mesa-commit
mailing list