From 0f3068a58bdbceb2cb93e3848b0e2145629cdf43 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 30 Apr 2013 15:15:21 -0700 Subject: [PATCH] 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 --- src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 ++- src/mesa/drivers/dri/i965/brw_vec4.h | 4 +- .../dri/i965/brw_vec4_live_variables.cpp | 75 +++++-------------- .../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 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; -- 2.30.2