nir/opt_peephole_select: Don't count some unary operations
authorIan Romanick <ian.d.romanick@intel.com>
Fri, 1 Nov 2019 21:52:38 +0000 (14:52 -0700)
committerIan Romanick <ian.d.romanick@intel.com>
Tue, 3 Dec 2019 00:46:19 +0000 (16:46 -0800)
In many cases, fsat, fneg, fabs, ineg, and iabs will get folded into
another instruction as either source or destination modifiers.
Counting them as instructions means that some if-statements won't get
converted to selects.  For example,

        vec1 32 ssa_25 = flt32 ssa_0, ssa_23.x
        /* succs: block_1 block_2 */
        if ssa_25 {
                block block_1:
                /* preds: block_0 */
                vec1 32 ssa_26 = fabs ssa_24
                vec1 32 ssa_27 = fneg ssa_26
                vec1 32 ssa_28 = fabs ssa_20
                vec1 32 ssa_29 = fneg ssa_28
                vec1 32 ssa_30 = fmul ssa_27, ssa_29
                vec1 32 ssa_31 = fsat ssa_30
                /* succs: block_3 */
        } else {
                block block_2:
                /* preds: block_0 */
                /* succs: block_3 */
        }
        block block_3:
        /* preds: block_1 block_2 */

block_1 isn't really 6 instructions, but it will be counted that way.

Most callers of the peephole_select pass use either 1 or 8.  It's very
easy to blow way past either of these limits with things that are really
only one or two actual instructions.

I also tried some fancier things like making sure the fsat was of
another SSA def from the same block, but the simple test was actually
better.

The i965 back-end SEL peephole pass still helps ~700 shaders in
shader-db with this change.

Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
All Gen6+ platforms had similar results. (Ice Lake shown)
total instructions in shared programs: 14743694 -> 14738910 (-0.03%)
instructions in affected programs: 156575 -> 151791 (-3.06%)
helped: 1204
HURT: 0
helped stats (abs) min: 1 max: 27 x̄: 3.97 x̃: 3
helped stats (rel) min: 0.15% max: 19.57% x̄: 5.15% x̃: 4.55%
95% mean confidence interval for instructions value: -4.12 -3.82
95% mean confidence interval for instructions %-change: -5.35% -4.95%
Instructions are helped.

total cycles in shared programs: 231749141 -> 231602916 (-0.06%)
cycles in affected programs: 2818975 -> 2672750 (-5.19%)
helped: 876
HURT: 322
helped stats (abs) min: 2 max: 788 x̄: 180.99 x̃: 220
helped stats (rel) min: <.01% max: 43.82% x̄: 20.75% x̃: 19.44%
HURT stats (abs)   min: 1 max: 1188 x̄: 38.27 x̃: 20
HURT stats (rel)   min: 0.09% max: 102.67% x̄: 5.17% x̃: 1.70%
95% mean confidence interval for cycles value: -130.47 -113.64
95% mean confidence interval for cycles %-change: -14.85% -12.72%
Cycles are helped.

total sends in shared programs: 730495 -> 730491 (<.01%)
sends in affected programs: 46 -> 42 (-8.70%)
helped: 2
HURT: 0

Iron Lake and GM45 had similar results. (Iron Lake shown)
total instructions in shared programs: 8122757 -> 8122617 (<.01%)
instructions in affected programs: 14716 -> 14576 (-0.95%)
helped: 46
HURT: 1
helped stats (abs) min: 1 max: 8 x̄: 3.07 x̃: 3
helped stats (rel) min: 0.36% max: 10.00% x̄: 2.54% x̃: 1.06%
HURT stats (abs)   min: 1 max: 1 x̄: 1.00 x̃: 1
HURT stats (rel)   min: 1.59% max: 1.59% x̄: 1.59% x̃: 1.59%
95% mean confidence interval for instructions value: -3.42 -2.54
95% mean confidence interval for instructions %-change: -3.28% -1.62%
Instructions are helped.

total cycles in shared programs: 188510100 -> 188509780 (<.01%)
cycles in affected programs: 58994 -> 58674 (-0.54%)
helped: 32
HURT: 1
helped stats (abs) min: 2 max: 96 x̄: 10.06 x̃: 6
helped stats (rel) min: 0.05% max: 15.29% x̄: 1.37% x̃: 0.31%
HURT stats (abs)   min: 2 max: 2 x̄: 2.00 x̃: 2
HURT stats (rel)   min: 0.68% max: 0.68% x̄: 0.68% x̃: 0.68%
95% mean confidence interval for cycles value: -16.34 -3.06
95% mean confidence interval for cycles %-change: -2.46% -0.15%
Cycles are helped.

src/compiler/nir/nir_opt_peephole_select.c

index 14f36e7..869f663 100644 (file)
@@ -27,6 +27,7 @@
 
 #include "nir.h"
 #include "nir_control_flow.h"
+#include "nir_search_helpers.h"
 
 /*
  * Implements a small peephole optimization that looks for
@@ -107,6 +108,8 @@ block_check_for_allowed_instrs(nir_block *block, unsigned *count,
 
       case nir_instr_type_alu: {
          nir_alu_instr *mov = nir_instr_as_alu(instr);
+         bool movelike = false;
+
          switch (mov->op) {
          case nir_op_mov:
          case nir_op_fneg:
@@ -116,6 +119,7 @@ block_check_for_allowed_instrs(nir_block *block, unsigned *count,
          case nir_op_vec2:
          case nir_op_vec3:
          case nir_op_vec4:
+            movelike = true;
             break;
 
          case nir_op_fcos:
@@ -148,8 +152,18 @@ block_check_for_allowed_instrs(nir_block *block, unsigned *count,
          if (!mov->dest.dest.is_ssa)
             return false;
 
+         const struct nir_block *const expected_block = mov->instr.block;
+         const nir_alu_type expected_type =
+            nir_alu_type_get_base_type(nir_op_infos[mov->op].output_type);
+
          if (alu_ok) {
-            (*count)++;
+            /* If the ALU operation is an fsat or a move-like operation, do
+             * not count it.  The expectation is that it will eventually be
+             * merged as a destination modifier or source modifier on some
+             * other instruction.
+             */
+            if (mov->op != nir_op_fsat && !movelike)
+               (*count)++;
          } else {
             /* Can't handle saturate */
             if (mov->dest.saturate)