[Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
From: Kenneth Graunke
The pass breaks live ranges of virtual registers by allocating new
registers when it sees an assignment to a virtual GRF it's already seen
written.
total instructions in shared programs: 1656292 -> 1651898 (-0.27%)
instructions in affected programs: 466836 -> 462442 (-0.94%)
GAINED:173
LOST: 3
---
[mattst88]: Perform renaming at control flow level 0, rather than bailing
at first sight of control flow.
src/mesa/drivers/dri/i965/brw_fs.cpp | 56
src/mesa/drivers/dri/i965/brw_fs.h | 1 +
2 files changed, 57 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index dbce167..aa957cc 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -2076,6 +2076,61 @@ fs_visitor::opt_algebraic()
}
bool
+fs_visitor::opt_register_renaming()
+{
+ bool progress = false;
+ int depth = 0;
+
+ int remap[virtual_grf_count];
+ memset(remap, -1, sizeof(int) * virtual_grf_count);
+
+ foreach_list(node, &this->instructions) {
+ fs_inst *inst = (fs_inst *) node;
+
+ if (inst->opcode == BRW_OPCODE_IF || inst->opcode == BRW_OPCODE_DO) {
+ depth++;
+ } else if (inst->opcode == BRW_OPCODE_ENDIF ||
+ inst->opcode == BRW_OPCODE_WHILE) {
+ depth--;
+ }
+
+ /* Rewrite instruction sources. */
+ for (int i = 0; i < 3; i++) {
+ if (inst->src[i].file == GRF &&
+ remap[inst->src[i].reg] != -1 &&
+ remap[inst->src[i].reg] != inst->src[i].reg) {
+inst->src[i].reg = remap[inst->src[i].reg];
+progress = true;
+ }
+ }
+
+ int dst = inst->dst.reg;
+
+ if (depth == 0 &&
+ inst->dst.file == GRF &&
+ virtual_grf_sizes[inst->dst.reg] == 1 &&
+ !inst->is_partial_write()) {
+ if (remap[dst] == -1) {
+remap[dst] = dst;
+ } else {
+remap[dst] = virtual_grf_alloc(virtual_grf_sizes[dst]);
+inst->dst.reg = remap[dst];
+progress = true;
+ }
+ } else if (inst->dst.file == GRF &&
+ remap[dst] != -1 &&
+ remap[dst] != dst) {
+ inst->dst.reg = remap[dst];
+ progress = true;
+ }
+ }
+
+ invalidate_live_intervals();
+
+ return progress;
+}
+
+bool
fs_visitor::compute_to_mrf()
{
bool progress = false;
@@ -3031,6 +3086,7 @@ fs_visitor::run()
progress = opt_peephole_sel() || progress;
progress = dead_control_flow_eliminate(this) || progress;
progress = opt_saturate_propagation() || progress;
+ progress = opt_register_renaming() || progress;
progress = register_coalesce() || progress;
progress = compute_to_mrf() || progress;
} while (progress);
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
b/src/mesa/drivers/dri/i965/brw_fs.h
index acf6a11..3f368c2 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -371,6 +371,7 @@ public:
bool opt_copy_propagate_local(void *mem_ctx, bblock_t *block,
exec_list *acp);
void opt_drop_redundant_mov_to_flags();
+ bool opt_split_live_ranges();
bool register_coalesce();
bool compute_to_mrf();
bool dead_code_eliminate();
--
1.8.3.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
From: Kenneth Graunke
The pass breaks live ranges of virtual registers by allocating new
registers when it sees an assignment to a virtual GRF it's already seen
written.
total instructions in shared programs: 4337879 -> 4335014 (-0.07%)
instructions in affected programs: 343865 -> 341000 (-0.83%)
GAINED:46
LOST: 1
Reviewed-by: Eric Anholt
Reviewed-by: Matt Turner
---
src/mesa/drivers/dri/i965/brw_fs.cpp | 67
src/mesa/drivers/dri/i965/brw_fs.h | 1 +
2 files changed, 68 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index cc5ec16..6ec1c8f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -2068,6 +2068,72 @@ fs_visitor::opt_algebraic()
}
bool
+fs_visitor::opt_register_renaming()
+{
+ bool progress = false;
+ int depth = 0;
+
+ int remap[virtual_grf_count];
+ memset(remap, -1, sizeof(int) * virtual_grf_count);
+
+ foreach_in_list(fs_inst, inst, &this->instructions) {
+ if (inst->opcode == BRW_OPCODE_IF || inst->opcode == BRW_OPCODE_DO) {
+ depth++;
+ } else if (inst->opcode == BRW_OPCODE_ENDIF ||
+ inst->opcode == BRW_OPCODE_WHILE) {
+ depth--;
+ }
+
+ /* Rewrite instruction sources. */
+ for (int i = 0; i < inst->sources; i++) {
+ if (inst->src[i].file == GRF &&
+ remap[inst->src[i].reg] != -1 &&
+ remap[inst->src[i].reg] != inst->src[i].reg) {
+inst->src[i].reg = remap[inst->src[i].reg];
+progress = true;
+ }
+ }
+
+ int dst = inst->dst.reg;
+
+ if (depth == 0 &&
+ inst->dst.file == GRF &&
+ virtual_grf_sizes[inst->dst.reg] == 1 &&
+ !inst->is_partial_write()) {
+ if (remap[dst] == -1) {
+remap[dst] = dst;
+ } else {
+remap[dst] = virtual_grf_alloc(virtual_grf_sizes[dst]);
+inst->dst.reg = remap[dst];
+progress = true;
+ }
+ } else if (inst->dst.file == GRF &&
+ remap[dst] != -1 &&
+ remap[dst] != dst) {
+ inst->dst.reg = remap[dst];
+ progress = true;
+ }
+ }
+
+ if (progress) {
+ invalidate_live_intervals();
+
+ for (unsigned i = 0; i < ARRAY_SIZE(delta_x); i++) {
+ if (delta_x[i].file == GRF && remap[delta_x[i].reg] != -1) {
+delta_x[i].reg = remap[delta_x[i].reg];
+ }
+ }
+ for (unsigned i = 0; i < ARRAY_SIZE(delta_y); i++) {
+ if (delta_y[i].file == GRF && remap[delta_y[i].reg] != -1) {
+delta_y[i].reg = remap[delta_y[i].reg];
+ }
+ }
+ }
+
+ return progress;
+}
+
+bool
fs_visitor::compute_to_mrf()
{
bool progress = false;
@@ -3092,6 +3158,7 @@ fs_visitor::run()
OPT(dead_code_eliminate);
OPT(opt_peephole_sel);
OPT(dead_control_flow_eliminate, this);
+ OPT(opt_register_renaming);
OPT(opt_saturate_propagation);
OPT(register_coalesce);
OPT(compute_to_mrf);
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
b/src/mesa/drivers/dri/i965/brw_fs.h
index e7a82c4..d30b0b8 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -339,6 +339,7 @@ public:
bool opt_copy_propagate_local(void *mem_ctx, bblock_t *block,
exec_list *acp);
void opt_drop_redundant_mov_to_flags();
+ bool opt_register_renaming();
bool register_coalesce();
bool compute_to_mrf();
bool dead_code_eliminate();
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
On Mon, Apr 14, 2014 at 3:01 PM, Matt Turner wrote:
> From: Kenneth Graunke
>
> The pass breaks live ranges of virtual registers by allocating new
> registers when it sees an assignment to a virtual GRF it's already seen
> written.
>
> total instructions in shared programs: 1656292 -> 1651898 (-0.27%)
> instructions in affected programs: 466836 -> 462442 (-0.94%)
> GAINED:173
> LOST: 3
> ---
> [mattst88]: Perform renaming at control flow level 0, rather than bailing
> at first sight of control flow.
>
> src/mesa/drivers/dri/i965/brw_fs.cpp | 56
>
> src/mesa/drivers/dri/i965/brw_fs.h | 1 +
> 2 files changed, 57 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index dbce167..aa957cc 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2076,6 +2076,61 @@ fs_visitor::opt_algebraic()
> }
>
> bool
> +fs_visitor::opt_register_renaming()
> +{
> + bool progress = false;
> + int depth = 0;
> +
> + int remap[virtual_grf_count];
> + memset(remap, -1, sizeof(int) * virtual_grf_count);
> +
> + foreach_list(node, &this->instructions) {
> + fs_inst *inst = (fs_inst *) node;
> +
> + if (inst->opcode == BRW_OPCODE_IF || inst->opcode == BRW_OPCODE_DO) {
> + depth++;
> + } else if (inst->opcode == BRW_OPCODE_ENDIF ||
> + inst->opcode == BRW_OPCODE_WHILE) {
> + depth--;
> + }
> +
> + /* Rewrite instruction sources. */
> + for (int i = 0; i < 3; i++) {
> + if (inst->src[i].file == GRF &&
> + remap[inst->src[i].reg] != -1 &&
> + remap[inst->src[i].reg] != inst->src[i].reg) {
> +inst->src[i].reg = remap[inst->src[i].reg];
> +progress = true;
> + }
> + }
> +
> + int dst = inst->dst.reg;
> +
> + if (depth == 0 &&
> + inst->dst.file == GRF &&
> + virtual_grf_sizes[inst->dst.reg] == 1 &&
> + !inst->is_partial_write()) {
> + if (remap[dst] == -1) {
> +remap[dst] = dst;
> + } else {
> +remap[dst] = virtual_grf_alloc(virtual_grf_sizes[dst]);
> +inst->dst.reg = remap[dst];
> +progress = true;
> + }
> + } else if (inst->dst.file == GRF &&
> + remap[dst] != -1 &&
> + remap[dst] != dst) {
> + inst->dst.reg = remap[dst];
> + progress = true;
> + }
> + }
> +
> + invalidate_live_intervals();
> +
> + return progress;
> +}
> +
> +bool
> fs_visitor::compute_to_mrf()
> {
> bool progress = false;
> @@ -3031,6 +3086,7 @@ fs_visitor::run()
> progress = opt_peephole_sel() || progress;
> progress = dead_control_flow_eliminate(this) || progress;
> progress = opt_saturate_propagation() || progress;
> + progress = opt_register_renaming() || progress;
> progress = register_coalesce() || progress;
> progress = compute_to_mrf() || progress;
>} while (progress);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index acf6a11..3f368c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -371,6 +371,7 @@ public:
> bool opt_copy_propagate_local(void *mem_ctx, bblock_t *block,
> exec_list *acp);
> void opt_drop_redundant_mov_to_flags();
> + bool opt_split_live_ranges();
Rebase fail. This should obviously be opt_register_renaming().
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
Matt Turner writes: > From: Kenneth Graunke > > The pass breaks live ranges of virtual registers by allocating new > registers when it sees an assignment to a virtual GRF it's already seen > written. > > total instructions in shared programs: 1656292 -> 1651898 (-0.27%) > instructions in affected programs: 466836 -> 462442 (-0.94%) > GAINED:173 > LOST: 3 > --- > [mattst88]: Perform renaming at control flow level 0, rather than bailing > at first sight of control flow. I think this needs to handle renaming the regs in special[] in compact_virtual_grfs(). Either that or we *don't* have to actually do that work and we should drop it from that code, too. pgpkCLPrfhrSR.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
Eric Anholt writes: > Matt Turner writes: > >> From: Kenneth Graunke >> >> The pass breaks live ranges of virtual registers by allocating new >> registers when it sees an assignment to a virtual GRF it's already seen >> written. >> >> total instructions in shared programs: 1656292 -> 1651898 (-0.27%) >> instructions in affected programs: 466836 -> 462442 (-0.94%) >> GAINED:173 >> LOST: 3 >> --- >> [mattst88]: Perform renaming at control flow level 0, rather than bailing >> at first sight of control flow. > > I think this needs to handle renaming the regs in special[] in > compact_virtual_grfs(). Either that or we *don't* have to actually do > that work and we should drop it from that code, too. Now that special[] isn't special any more, this patch is: Reviewed-by: Eric Anholt pgp_Av12xLzkt.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
On Wed, Aug 13, 2014 at 1:22 PM, Matt Turner wrote:
> From: Kenneth Graunke
>
> The pass breaks live ranges of virtual registers by allocating new
> registers when it sees an assignment to a virtual GRF it's already seen
> written.
>
> total instructions in shared programs: 4337879 -> 4335014 (-0.07%)
> instructions in affected programs: 343865 -> 341000 (-0.83%)
> GAINED:46
> LOST: 1
>
> Reviewed-by: Eric Anholt
> Reviewed-by: Matt Turner
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 67
>
> src/mesa/drivers/dri/i965/brw_fs.h | 1 +
> 2 files changed, 68 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index cc5ec16..6ec1c8f 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2068,6 +2068,72 @@ fs_visitor::opt_algebraic()
> }
>
> bool
> +fs_visitor::opt_register_renaming()
> +{
> + bool progress = false;
> + int depth = 0;
> +
> + int remap[virtual_grf_count];
> + memset(remap, -1, sizeof(int) * virtual_grf_count);
> +
> + foreach_in_list(fs_inst, inst, &this->instructions) {
> + if (inst->opcode == BRW_OPCODE_IF || inst->opcode == BRW_OPCODE_DO) {
> + depth++;
> + } else if (inst->opcode == BRW_OPCODE_ENDIF ||
> + inst->opcode == BRW_OPCODE_WHILE) {
> + depth--;
> + }
> +
> + /* Rewrite instruction sources. */
> + for (int i = 0; i < inst->sources; i++) {
> + if (inst->src[i].file == GRF &&
> + remap[inst->src[i].reg] != -1 &&
> + remap[inst->src[i].reg] != inst->src[i].reg) {
> +inst->src[i].reg = remap[inst->src[i].reg];
> +progress = true;
> + }
> + }
> +
> + int dst = inst->dst.reg;
> +
> + if (depth == 0 &&
> + inst->dst.file == GRF &&
> + virtual_grf_sizes[inst->dst.reg] == 1 &&
We can just use dst here instead of inst->dst.reg
> + !inst->is_partial_write()) {
> + if (remap[dst] == -1) {
> +remap[dst] = dst;
> + } else {
> +remap[dst] = virtual_grf_alloc(virtual_grf_sizes[dst]);
Since this code only gets run when virtual_grf_sizes[dst] == 1, we can
just do virtual_grf_alloc(1).
Also, I think you're forgetting to set the type of the new destination
(i.e. remap[dst]).
Finally, I don't think this should matter in practice, but if the
instruction's destination is a hardware register we should probably
bail out.
> +inst->dst.reg = remap[dst];
> +progress = true;
> + }
> + } else if (inst->dst.file == GRF &&
> + remap[dst] != -1 &&
> + remap[dst] != dst) {
> + inst->dst.reg = remap[dst];
I think you're forgetting to set the type here too.
> + progress = true;
> + }
> + }
> +
> + if (progress) {
> + invalidate_live_intervals();
> +
> + for (unsigned i = 0; i < ARRAY_SIZE(delta_x); i++) {
> + if (delta_x[i].file == GRF && remap[delta_x[i].reg] != -1) {
> +delta_x[i].reg = remap[delta_x[i].reg];
> + }
> + }
> + for (unsigned i = 0; i < ARRAY_SIZE(delta_y); i++) {
> + if (delta_y[i].file == GRF && remap[delta_y[i].reg] != -1) {
> +delta_y[i].reg = remap[delta_y[i].reg];
> + }
> + }
> + }
> +
> + return progress;
> +}
> +
> +bool
> fs_visitor::compute_to_mrf()
> {
> bool progress = false;
> @@ -3092,6 +3158,7 @@ fs_visitor::run()
> OPT(dead_code_eliminate);
> OPT(opt_peephole_sel);
> OPT(dead_control_flow_eliminate, this);
> + OPT(opt_register_renaming);
> OPT(opt_saturate_propagation);
> OPT(register_coalesce);
> OPT(compute_to_mrf);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index e7a82c4..d30b0b8 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -339,6 +339,7 @@ public:
> bool opt_copy_propagate_local(void *mem_ctx, bblock_t *block,
> exec_list *acp);
> void opt_drop_redundant_mov_to_flags();
> + bool opt_register_renaming();
> bool register_coalesce();
> bool compute_to_mrf();
> bool dead_code_eliminate();
> --
> 1.8.5.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Other than my comments above,
Reviewed-by: Connor Abbott
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: Add pass to rename registers to break live ranges.
On Wed, Aug 13, 2014 at 8:36 PM, Connor Abbott wrote:
> On Wed, Aug 13, 2014 at 1:22 PM, Matt Turner wrote:
>> From: Kenneth Graunke
>>
>> The pass breaks live ranges of virtual registers by allocating new
>> registers when it sees an assignment to a virtual GRF it's already seen
>> written.
>>
>> total instructions in shared programs: 4337879 -> 4335014 (-0.07%)
>> instructions in affected programs: 343865 -> 341000 (-0.83%)
>> GAINED:46
>> LOST: 1
>>
>> Reviewed-by: Eric Anholt
>> Reviewed-by: Matt Turner
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 67
>>
>> src/mesa/drivers/dri/i965/brw_fs.h | 1 +
>> 2 files changed, 68 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index cc5ec16..6ec1c8f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -2068,6 +2068,72 @@ fs_visitor::opt_algebraic()
>> }
>>
>> bool
>> +fs_visitor::opt_register_renaming()
>> +{
>> + bool progress = false;
>> + int depth = 0;
>> +
>> + int remap[virtual_grf_count];
>> + memset(remap, -1, sizeof(int) * virtual_grf_count);
>> +
>> + foreach_in_list(fs_inst, inst, &this->instructions) {
>> + if (inst->opcode == BRW_OPCODE_IF || inst->opcode == BRW_OPCODE_DO) {
>> + depth++;
>> + } else if (inst->opcode == BRW_OPCODE_ENDIF ||
>> + inst->opcode == BRW_OPCODE_WHILE) {
>> + depth--;
>> + }
>> +
>> + /* Rewrite instruction sources. */
>> + for (int i = 0; i < inst->sources; i++) {
>> + if (inst->src[i].file == GRF &&
>> + remap[inst->src[i].reg] != -1 &&
>> + remap[inst->src[i].reg] != inst->src[i].reg) {
>> +inst->src[i].reg = remap[inst->src[i].reg];
>> +progress = true;
>> + }
>> + }
>> +
>> + int dst = inst->dst.reg;
>> +
>> + if (depth == 0 &&
>> + inst->dst.file == GRF &&
>> + virtual_grf_sizes[inst->dst.reg] == 1 &&
>
> We can just use dst here instead of inst->dst.reg
Yeah, a bit weird to use inst->dst.file in one conditional, and not
use inst->dst.reg in the next though.
>> + !inst->is_partial_write()) {
>> + if (remap[dst] == -1) {
>> +remap[dst] = dst;
>> + } else {
>> +remap[dst] = virtual_grf_alloc(virtual_grf_sizes[dst]);
>
> Since this code only gets run when virtual_grf_sizes[dst] == 1, we can
> just do virtual_grf_alloc(1).
>
> Also, I think you're forgetting to set the type of the new destination
> (i.e. remap[dst]).
>
> Finally, I don't think this should matter in practice, but if the
> instruction's destination is a hardware register we should probably
> bail out.
HW_REG's register file and number are stored in the fixed_hw_reg
struct, not in the regular file/reg fields.
>> +inst->dst.reg = remap[dst];
>> +progress = true;
>> + }
>> + } else if (inst->dst.file == GRF &&
>> + remap[dst] != -1 &&
>> + remap[dst] != dst) {
>> + inst->dst.reg = remap[dst];
>
> I think you're forgetting to set the type here too.
remap[] is just an array of the register numbers, so we're not
overwriting the type of inst->dst at any point, just giving it a new
register number.
Thanks for the review!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
