Matt Turner <matts...@gmail.com> writes: > 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 > new file mode 100644 > index 0000000..6addbb3 > --- /dev/null > +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> +
> +/** @file brw_fs_dead_code_eliminate.cpp
> + */
Perhaps some actual comment:
* 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;
> +
> + cfg_t cfg(&instructions);
> +
> + calculate_live_intervals();
> +
> + int num_vars = live_intervals->num_vars;
> + BITSET_WORD *live = ralloc_array(NULL, BITSET_WORD,
> BITSET_WORDS(num_vars));
> +
> + for (int b = 0; b < cfg.num_blocks; b++) {
> + bblock_t *block = cfg.blocks[b];
> + memcpy(live, live_intervals->bd[b].liveout,
> + sizeof(BITSET_WORD) * BITSET_WORDS(num_vars));
> +
> + for (fs_inst *inst = (fs_inst *)block->end;
> + inst != block->start->prev;
> + inst = (fs_inst *)inst->prev) {
> + if (inst->dst.file == GRF &&
> + !inst->has_side_effects() &&
> + !inst->writes_flag()) {
> + bool result_live = false;
> +
> + 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_vgrf[inst->dst.reg];
> + for (int i = 0; i < inst->regs_written; i++) {
> + result_live = result_live || BITSET_TEST(live, var + i);
> + }
> + }
You could just use the else block of this if statement all the time,
right? Seems easier.
> + if (!result_live) {
> + progress = true;
> +
> + switch (inst->opcode) {
> + case BRW_OPCODE_ADDC:
> + case BRW_OPCODE_SUBB:
> + case BRW_OPCODE_MACH:
> + inst->dst = fs_reg(retype(brw_null_reg(), inst->dst.type));
> + break;
> + default:
> + inst->opcode = BRW_OPCODE_NOP;
> + break;
> + }
> + continue;
I don't think the continue here is quite right: you'll skip looking at
the src args for a nulled-destination ADDC/SUBB/MACH. I think the
continue would still be appropriate in the default case.
> + }
> + }
> +
> + for (int i = 0; i < 3; i++) {
> + if (inst->src[i].file == GRF) {
> + int var = live_intervals->var_from_vgrf[inst->src[i].reg];
> +
> + for (int j = 0; j < inst->regs_read(this, i); j++) {
> + BITSET_SET(live, var + inst->src[i].reg_offset + j);
> + }
> + }
> + }
> + }
> + }
> +
> + ralloc_free(live);
> +
> + foreach_list_safe(node, &this->instructions) {
> + fs_inst *inst = (fs_inst *)node;
> +
> + if (inst->opcode == BRW_OPCODE_NOP) {
> + inst->remove();
> + }
> + }
Tiny optimization: this block could go under if (progress).
> + if (progress)
> + invalidate_live_intervals();
> +
> + return progress;
The "continue" comment is the only important one, I think. Anything
else you can take or leave, and this series is:
Reviewed-by: Eric Anholt <e...@anholt.net>
pgpKavHPi_FCw.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
