From: Francisco Jerez <currojerez@riseup.net>
Date: Sun, 20 Oct 2013 21:02:08 +0000 (-0700)
Subject: i965: Add a 'has_side_effects' back-end instruction predicate.
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=34fe051e215107dddbaae71e2edf15f88d839936;p=mesa.git

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 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