[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