[Mesa-dev] [PATCH 10.5/15] i965/fs: Extend SEL peephole to handle only matching MOVs.

[Matt Turner]

Before this patch, the following code would not be optimized even though
the final two instructions were common to the then and else blocks:

   (+f0) IF
   MOV dst2 ...
   MOV dst1 ...
   MOV dst0 ...
   ELSE
   MOV dst3 ...
   MOV dst1 ...
   MOV dst0 ...
   ENDIF

This commit extends the peephole to handle this case.

total instructions in shared programs: 1362275 -> 1361817 (-0.03%)
instructions in affected programs:     35527 -> 35069 (-1.29%)
---
Split mistakenly squashed patches.

 src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp | 93 +++++++++++++++++------
 1 file changed, 71 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
index 11c3677..8638f43 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
@@ -34,32 +34,84 @@
 #define MAX_MOVS 8 /**< The maximum number of MOVs to attempt to match. */
 
 /**
+ * For a given control flow graph <cfg> and number <start_block> of the basic
+ * block ending with an ENDIF instruction, return pointers to the associated
+ * IF and ELSE instructions.
+ *
+ * If no ELSE instruction is found before the associated IF, return false.
+ * Otherwise return true.
+ */
+static bool
+find_if_else_from_endif(const cfg_t *cfg, int start_block,
+                        fs_inst **if_inst, fs_inst **else_inst)
+{
+   assert(start_block > 0);
+   assert(if_inst);
+   assert(else_inst);
+
+   *if_inst = NULL;
+   *else_inst = NULL;
+
+   int depth = 0;
+   for (int b = start_block - 1; b >= 0; b--) {
+      bblock_t *block = cfg->blocks[b];
+
+      switch (block->end->opcode) {
+      case BRW_OPCODE_IF:
+         if (depth == 0) {
+            *if_inst = (fs_inst *) block->end;
+            return *else_inst != NULL;
+         }
+         depth--;
+         break;
+      case BRW_OPCODE_ELSE:
+         if (depth == 0) {
+            *else_inst = (fs_inst *) block->end;
+         }
+         /* No change in depth */
+         break;
+      case BRW_OPCODE_ENDIF:
+      case BRW_OPCODE_WHILE:
+         depth++;
+         break;
+      case BRW_OPCODE_DO:
+         depth--;
+         break;
+      default:
+         break;
+      }
+   }
+
+   return false;
+}
+
+/**
  * Scans backwards from an ENDIF counting MOV instructions with common
  * destinations inside the "then" and "else" blocks of the if statement.
  *
- * A pointer to the fs_inst* for ENDIF is passed as the <match> argument. The
- * function stores pointers to the MOV instructions in the <then_mov> and
- * <else_mov> arrays. If the function is successful, the <match> points to the
- * fs_inst* pointing to the IF instruction at the beginning of the block.
+ * A pointer to the fs_inst* for ENDIF is passed as the <endif_inst> argument.
+ * The function stores pointers to the MOV instructions in the <then_mov> and
+ * <else_mov> arrays.
  *
  * \return the number of MOVs to a common destination found in the two branches
- *         or zero if an error occurred.
  *
  * E.g.:
- *    match       = IF ...
+ *                  IF ...
+ *    then_mov[2] = MOV g2, ...
  *    then_mov[1] = MOV g4, ...
  *    then_mov[0] = MOV g5, ...
  *                  ELSE ...
+ *    then_mov[2] = MOV g3, ...
  *    else_mov[1] = MOV g4, ...
  *    else_mov[0] = MOV g5, ...
  *                  ENDIF
- *    returns 2.
+ *    returns 2 (since only the first two MOVs have a common destination)
  */
 static int
 match_movs_from_endif(fs_inst *then_mov[MAX_MOVS], fs_inst *else_mov[MAX_MOVS],
-                      fs_inst **match)
+                      fs_inst *endif_inst, fs_inst *else_inst)
 {
-   fs_inst *m = *match;
+   fs_inst *m = endif_inst;
 
    assert(m->opcode == BRW_OPCODE_ENDIF);
    m = (fs_inst *) m->prev;
@@ -71,9 +123,7 @@ match_movs_from_endif(fs_inst *then_mov[MAX_MOVS], fs_inst *else_mov[MAX_MOVS],
       else_movs++;
    }
 
-   if (m->opcode != BRW_OPCODE_ELSE)
-      return 0;
-   m = (fs_inst *) m->prev;
+   m = (fs_inst *) else_inst->prev;
 
    int then_movs = 0;
    while (then_movs < MAX_MOVS && m->opcode == BRW_OPCODE_MOV) {
@@ -82,10 +132,6 @@ match_movs_from_endif(fs_inst *then_mov[MAX_MOVS], fs_inst *else_mov[MAX_MOVS],
       then_movs++;
    }
 
-   if (m->opcode != BRW_OPCODE_IF)
-      return 0;
-
-   *match = m;
    return MIN2(then_movs, else_movs);
 }
 
@@ -134,7 +180,7 @@ fs_visitor::opt_peephole_sel()
       bblock_t *block = cfg.blocks[b];
 
       int movs;
-      fs_inst *if_inst, *endif_inst;
+      fs_inst *if_inst, *else_inst, *endif_inst;
       fs_inst *start;
       fs_inst *else_mov[MAX_MOVS] = { NULL };
       fs_inst *then_mov[MAX_MOVS] = { NULL };
@@ -145,14 +191,17 @@ fs_visitor::opt_peephole_sel()
        */
       start = (fs_inst *) block->end;
       if (start->opcode == BRW_OPCODE_ENDIF) {
-         fs_inst *match = endif_inst = start;
+         endif_inst = start;
+
+         /* Find the associated IF and ELSE instructions for our ENDIF. */
+         if (!find_if_else_from_endif(&cfg, b, &if_inst, &else_inst))
+            continue;
 
          /* Find MOVs to a common destination. */
-         movs = match_movs_from_endif(then_mov, else_mov, &match);
+         movs = match_movs_from_endif(then_mov, else_mov, start, else_inst);
+
          if (movs == 0)
             continue;
-
-         if_inst = match;
       } else {
          continue;
       }
@@ -171,7 +220,7 @@ fs_visitor::opt_peephole_sel()
          if (!then_mov[i]->dst.equals(else_mov[i]->dst) ||
              then_mov[i]->is_partial_write() ||
              else_mov[i]->is_partial_write()) {
-            bb_progress = false;
+            movs = i;
             break;
          }
 
-- 
1.8.3.2