From 34fe051e215107dddbaae71e2edf15f88d839936 Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Sun, 20 Oct 2013 14:02:08 -0700 Subject: [PATCH] i965: Add a 'has_side_effects' back-end instruction predicate. This patch fixes the three dead code elimination passes and the VEC4/FS instruction scheduling passes so they leave instructions with side effects alone. At some point it might be interesting to have the instruction scheduler calculate the exact memory dependencies between atomic ops, but they're rare enough that it seems unlikely that it will make any practical difference. Reviewed-by: Paul Berry --- src/mesa/drivers/dri/i965/brw_fs.cpp | 25 ++++++++----------- .../dri/i965/brw_schedule_instructions.cpp | 6 ++++- src/mesa/drivers/dri/i965/brw_shader.cpp | 11 ++++++++ src/mesa/drivers/dri/i965/brw_shader.h | 7 ++++++ src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +- 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 7caa52ded91..c332d4cbb53 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -2059,7 +2059,7 @@ fs_visitor::dead_code_eliminate() foreach_list_safe(node, &this->instructions) { fs_inst *inst = (fs_inst *)node; - if (inst->dst.file == GRF) { + if (inst->dst.file == GRF && !inst->has_side_effects()) { bool dead = true; for (int i = 0; i < inst->regs_written; i++) { @@ -2220,31 +2220,26 @@ fs_visitor::dead_code_eliminate_local() get_dead_code_hash_entry(ht, inst->dst.reg, inst->dst.reg_offset); - if (inst->is_partial_write()) { - /* For a partial write, we can't remove any previous dead code - * candidate, since we're just modifying their result, but we can - * be dead code eliminiated ourselves. - */ - if (entry) { - entry->data = inst; + if (entry) { + if (inst->is_partial_write()) { + /* For a partial write, we can't remove any previous dead code + * candidate, since we're just modifying their result. + */ } else { - insert_dead_code_hash(ht, inst->dst.reg, inst->dst.reg_offset, - inst); - } - } else { - if (entry) { /* We're completely updating a channel, and there was a * previous write to the channel that wasn't read. Kill it! */ fs_inst *inst = (fs_inst *)entry->data; inst->remove(); progress = true; - _mesa_hash_table_remove(ht, entry); } + _mesa_hash_table_remove(ht, entry); + } + + if (!inst->has_side_effects()) insert_dead_code_hash(ht, inst->dst.reg, inst->dst.reg_offset, inst); - } } } } diff --git a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp index 5dcd0e8b881..fe0de080544 100644 --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp @@ -603,7 +603,8 @@ fs_instruction_scheduler::calculate_deps() schedule_node *n = (schedule_node *)node; fs_inst *inst = (fs_inst *)n->inst; - if (inst->opcode == FS_OPCODE_PLACEHOLDER_HALT) + if (inst->opcode == FS_OPCODE_PLACEHOLDER_HALT || + inst->has_side_effects()) add_barrier_deps(n); /* read-after-write deps. */ @@ -832,6 +833,9 @@ vec4_instruction_scheduler::calculate_deps() schedule_node *n = (schedule_node *)node; vec4_instruction *inst = (vec4_instruction *)n->inst; + if (inst->has_side_effects()) + add_barrier_deps(n); + /* read-after-write deps. */ for (int i = 0; i < 3; i++) { if (inst->src[i].file == GRF) { diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp index b7b0a5b181d..ddb45241d83 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp @@ -602,6 +602,17 @@ backend_instruction::can_do_source_mods() } } +bool +backend_instruction::has_side_effects() const +{ + switch (opcode) { + case SHADER_OPCODE_UNTYPED_ATOMIC: + return true; + default: + return false; + } +} + void backend_visitor::dump_instructions() { diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h index cc76a44493b..88c23115e08 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.h +++ b/src/mesa/drivers/dri/i965/brw_shader.h @@ -46,6 +46,13 @@ public: bool is_control_flow(); bool can_do_source_mods(); + /** + * True if the instruction has side effects other than writing to + * its destination registers. You are expected not to reorder or + * optimize these out unless you know what you are doing. + */ + bool has_side_effects() const; + enum opcode opcode; /* BRW_OPCODE_* or FS_OPCODE_* */ uint32_t predicate; diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp index 32259124f4d..20fbd4578d6 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp @@ -318,7 +318,7 @@ vec4_visitor::dead_code_eliminate() foreach_list_safe(node, &this->instructions) { vec4_instruction *inst = (vec4_instruction *)node; - if (inst->dst.file == GRF) { + if (inst->dst.file == GRF && !inst->has_side_effects()) { assert(this->virtual_grf_end[inst->dst.reg] >= pc); if (this->virtual_grf_end[inst->dst.reg] == pc) { /* Don't dead code eliminate instructions that write to the -- 2.30.2