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 <stereotype441@gmail.com>

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 7caa52ded91878d90151a43b517383a85faf485a..c332d4cbb539ea2e3156b0f27baa0dd8d5a38f18 100644 (file)
--- 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 5dcd0e8b88149d8eec40ad0fd5827fc3a1a2afa2..fe0de0805444bd74c07e8b4edac4dbe55eebb466 100644 (file)
--- 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 b7b0a5b181d924148bb0c580df089fae18f438f4..ddb45241d8305a5109fcb961196b9e49876f8968 100644 (file)
--- 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 cc76a44493b8a8af223bffcc2bfbde03b7a8680b..88c23115e08c4a5b3b51a011e78a78a4f62a6362 100644 (file)
--- 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 32259124f4d06198a3161f4bf20698ea3fe4baab..20fbd4578d6b3a4d087721e5c978b1455b33b44c 100644 (file)
--- 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