[Mesa-dev] [PATCH 03/15] i965/cfg: Rework to make IF & ELSE blocks flow into ENDIF.

[Matt Turner]

Previously we made the basic block following an ENDIF instruction a
successor of the basic blocks ending with IF and ELSE. The PRM says that
IF and ELSE instructions jump *to* the ENDIF, rather than over it.

This should be immaterial to dataflow analysis, except for if, break,
endif sequences:

   START B1 <-B0 <-B9
0x00000100: cmp.g.f0(8)     null            g15<8,8,1>F     g4<0,1,0>F
0x00000110: (+f0) if(8) 0 0                 null            0x00000000UD
   END B1 ->B2 ->B4
   START B2 <-B1
   break
0x00000120: break(8) 0 0                    null            0D
   END B2 ->B10
   START B3
0x00000130: endif(8) 2                      null            0x00000002UD
   END B3 ->B4

The ENDIF block would have no parents, so dataflow analysis would
generate incorrect results, preventing copy propagation from eliminating
some instructions.

This patch changes the CFG to make ENDIF start rather than end basic
blocks, so that it can be the jump target of the IF and ELSE
instructions.

It helps three programs (including two fs8/fs16 pairs) and hurts a
single fs8/fs16 pair.

total instructions in shared programs: 1561126 -> 1561066 (-0.00%)
instructions in affected programs:     983 -> 923 (-6.10%)

More importantly, it allows copy propagation to handle more cases.
Disabling the register_coalesce() pass before this patch hurts 58
programs, while afterward it only hurts 11 programs.
---
 src/mesa/drivers/dri/i965/brw_cfg.cpp | 46 +++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp b/src/mesa/drivers/dri/i965/brw_cfg.cpp
index 548b458..83c3c34 100644
--- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
+++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
@@ -133,10 +133,7 @@ cfg_t::create(void *parent_mem_ctx, exec_list *instructions)
 
 	 cur_if = cur;
 	 cur_else = NULL;
-	 /* Set up the block just after the endif.  Don't know when exactly
-	  * it will start, yet.
-	  */
-	 cur_endif = new_block();
+         cur_endif = NULL;
 
 	 /* Set up our immediately following block, full of "then"
 	  * instructions.
@@ -149,26 +146,49 @@ cfg_t::create(void *parent_mem_ctx, exec_list *instructions)
 	 break;
 
       case BRW_OPCODE_ELSE:
-	 cur->add_successor(mem_ctx, cur_endif);
+         cur_else = cur;
 
 	 next = new_block();
 	 next->start = (backend_instruction *)inst->next;
 	 cur_if->add_successor(mem_ctx, next);
-	 cur_else = next;
 
 	 set_next_block(next);
 	 break;
 
       case BRW_OPCODE_ENDIF: {
-	 cur_endif->start = (backend_instruction *)inst->next;
-	 cur->add_successor(mem_ctx, cur_endif);
-	 set_next_block(cur_endif);
+         backend_instruction *prev_inst = (backend_instruction *)inst->prev;
+         switch (prev_inst->opcode) {
+         case BRW_OPCODE_IF:
+         case BRW_OPCODE_ELSE:
+         case BRW_OPCODE_WHILE:
+         case BRW_OPCODE_DO:
+         case BRW_OPCODE_BREAK:
+         case BRW_OPCODE_CONTINUE:
+            /* New block was just created; use it. */
+            cur_endif = cur;
+            break;
+         case BRW_OPCODE_ENDIF:
+         default:
+            cur_endif = new_block();
+            cur_endif->start = inst;
+
+            cur->end = prev_inst;
+            cur->add_successor(mem_ctx, cur_endif);
+
+            ip--;
+            set_next_block(cur_endif);
+            ip++;
+            break;
+         }
 
-	 if (!cur_else)
-	    cur_if->add_successor(mem_ctx, cur_endif);
+         backend_instruction *else_inst = NULL;
+         if (cur_else) {
+            else_inst = (backend_instruction *)cur_else->end;
 
-         backend_instruction *else_inst = cur_else ?
-            (backend_instruction *) cur_else->start->prev : NULL;
+            cur_else->add_successor(mem_ctx, cur_endif);
+         } else {
+            cur_if->add_successor(mem_ctx, cur_endif);
+         }
 
          assert(cur_if->end->opcode == BRW_OPCODE_IF);
          assert(!else_inst || else_inst->opcode == BRW_OPCODE_ELSE);
-- 
1.8.3.2