i965: Relax restriction on scheduling last instruction.
authorMatt Turner <mattst88@gmail.com>
Mon, 15 Feb 2016 18:05:33 +0000 (10:05 -0800)
committerMatt Turner <mattst88@gmail.com>
Thu, 31 Mar 2016 02:54:30 +0000 (19:54 -0700)
I think when this code was written, basic blocks were always ended by a
control flow instruction or an end-of-thread message. That's no longer
the case, and removing this restriction actually helps things:

   instructions in affected programs: 7267 -> 7244 (-0.32%)
   helped: 4

   total cycles in shared programs: 66559580 -> 66431900 (-0.19%)
   cycles in affected programs: 28310152 -> 28182472 (-0.45%)
   helped: 9577
   HURT: 879

   GAINED: 2

The addition of the is_control_flow() checks is not a functional change,
since the add_insts_from_block() does not put them in the list of
instructions to schedule. I plan to change this in a later patch.

Reviewed-by: Francisco Jerez <currojerez@riseup.net>
src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp

index 66eb07e6d1a73ebdbbcd356b23baa3e037d276dc..46b45a5ea0172a16b50105e73de0c814a251ab83 100644 (file)
@@ -923,15 +923,6 @@ fs_instruction_scheduler::calculate_deps()
     */
    schedule_node *last_fixed_grf_write = NULL;
 
-   /* The last instruction always needs to still be the last
-    * instruction.  Either it's flow control (IF, ELSE, ENDIF, DO,
-    * WHILE) and scheduling other things after it would disturb the
-    * basic block, or it's FB_WRITE and we should do a better job at
-    * dead code elimination anyway.
-    */
-   schedule_node *last = (schedule_node *)instructions.get_tail();
-   add_barrier_deps(last);
-
    memset(last_grf_write, 0, sizeof(last_grf_write));
    memset(last_mrf_write, 0, sizeof(last_mrf_write));
 
@@ -940,7 +931,8 @@ fs_instruction_scheduler::calculate_deps()
       fs_inst *inst = (fs_inst *)n->inst;
 
       if (inst->opcode == FS_OPCODE_PLACEHOLDER_HALT ||
-         inst->has_side_effects())
+          inst->is_control_flow() ||
+          inst->has_side_effects())
          add_barrier_deps(n);
 
       /* read-after-write deps. */
@@ -1166,15 +1158,6 @@ vec4_instruction_scheduler::calculate_deps()
     */
    schedule_node *last_fixed_grf_write = NULL;
 
-   /* The last instruction always needs to still be the last instruction.
-    * Either it's flow control (IF, ELSE, ENDIF, DO, WHILE) and scheduling
-    * other things after it would disturb the basic block, or it's the EOT
-    * URB_WRITE and we should do a better job at dead code eliminating
-    * anything that could have been scheduled after it.
-    */
-   schedule_node *last = (schedule_node *)instructions.get_tail();
-   add_barrier_deps(last);
-
    memset(last_grf_write, 0, sizeof(last_grf_write));
    memset(last_mrf_write, 0, sizeof(last_mrf_write));
 
@@ -1182,7 +1165,7 @@ vec4_instruction_scheduler::calculate_deps()
    foreach_in_list(schedule_node, n, &instructions) {
       vec4_instruction *inst = (vec4_instruction *)n->inst;
 
-      if (inst->has_side_effects())
+      if (inst->is_control_flow() || inst->has_side_effects())
          add_barrier_deps(n);
 
       /* read-after-write deps. */