On Wednesday, October 29, 2014 02:10:12 PM Matt Turner wrote: > Most prominently helps Natural Selection 2, which has a surprising > number shaders that do very complicated things before drawing black. > > instructions in affected programs: 23824 -> 19570 (-17.86%) > --- > .../dri/i965/brw_fs_dead_code_eliminate.cpp | 23 > +++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-)
(requoting the diff to add more context...)
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp index
> 9cf8d89..c5f5ede 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> @@ -21,93 +21,111 @@
>
> * IN THE SOFTWARE.
> */
>
> #include "brw_fs.h"
> #include "brw_fs_live_variables.h"
> #include "brw_cfg.h"
>
> /** @file brw_fs_dead_code_eliminate.cpp
> *
> * Dataflow-aware dead code elimination.
> *
> * Walks the instruction list from the bottom, removing instructions that
> * have results that both aren't used in later blocks and haven't been read
> * yet in the tail end of this block.
> */
>
> bool
> fs_visitor::dead_code_eliminate()
> {
>
> bool progress = false;
>
> calculate_live_intervals();
>
> int num_vars = live_intervals->num_vars;
> BITSET_WORD *live = ralloc_array(NULL, BITSET_WORD,
> BITSET_WORDS(num_vars));
> + BITSET_WORD *flag_live = ralloc_array(NULL, BITSET_WORD, 1);
>
> foreach_block (block, cfg) {
> memcpy(live, live_intervals->block_data[block->num].liveout,
> sizeof(BITSET_WORD) * BITSET_WORDS(num_vars));
> + memcpy(flag_live, live_intervals->block_data[block->num].flag_liveout,
> + sizeof(BITSET_WORD));
>
> foreach_inst_in_block_reverse(fs_inst, inst, block) {
> - if (inst->dst.file == GRF &&
> - !inst->has_side_effects() &&
> - !inst->writes_flag()) {
> + if (inst->dst.file == GRF && !inst->has_side_effects()) {
> bool result_live = false;
This seems wrong to me. Instructions handled here must have a destination, but
now
can also write the flag...yet...
> if (inst->regs_written == 1) {
> int var = live_intervals->var_from_reg(&inst->dst);
> result_live = BITSET_TEST(live, var);
> } else {
> int var = live_intervals->var_from_reg(&inst->dst);
> for (int i = 0; i < inst->regs_written; i++) {
> result_live = result_live || BITSET_TEST(live, var + i);
> }
> }
>
> if (!result_live) {
> progress = true;
>
> if (inst->writes_accumulator) {
> inst->dst = fs_reg(retype(brw_null_reg(),
> inst->dst.type));
> } else {
> inst->opcode = BRW_OPCODE_NOP;
> continue;
...here you NOP the instruction, without considering whether the flag value is
live.
I think you meant to change the (inst->writes_accumulator check to be
if (inst->writes_accumulator || inst->writes_flags())
that way, you just make the destination NULL, but leave it generating the flag
register...
> }
> }
> }
>
> + if (inst->dst.is_null() && inst->writes_flag()) {
> + if (!BITSET_TEST(flag_live, inst->flag_subreg)) {
> + inst->opcode = BRW_OPCODE_NOP;
> + progress = true;
> + continue;
> + }
> + }
...which this code block would clean up, NOP'ing instructions with a NULL
destination
and unused flag value.
With that fixed, it looks good to me.
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
> +
>
> if (inst->dst.file == GRF) {
> if (!inst->is_partial_write()) {
> int var = live_intervals->var_from_reg(&inst->dst);
> for (int i = 0; i < inst->regs_written; i++) {
> BITSET_CLEAR(live, var + i);
> }
> }
> }
>
> + if (inst->writes_flag()) {
> + BITSET_CLEAR(flag_live, inst->flag_subreg);
> + }
> +
>
> for (int i = 0; i < inst->sources; i++) {
> if (inst->src[i].file == GRF) {
> int var = live_intervals->var_from_reg(&inst->src[i]);
> for (int j = 0; j < inst->regs_read(this, i); j++) {
> BITSET_SET(live, var + j);
> }
> }
> }
>
> +
> + if (inst->reads_flag()) {
> + BITSET_SET(flag_live, inst->flag_subreg);
> + }
> }
> }
>
> ralloc_free(live);
> + ralloc_free(flag_live);
>
> if (progress) {
> foreach_block_and_inst_safe (block, backend_instruction, inst, cfg) {
> if (inst->opcode == BRW_OPCODE_NOP) {
> inst->remove(block);
> }
> }
>
> invalidate_live_intervals();
> }
>
> return progress;
> }
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
