[v2,2/3] intel/compiler: allow constant propagation to first source of 2src instructions

Submitted by Iago Toral Quiroga on March 4, 2019, 12:15 p.m.

Details

Message ID 20190304121544.14044-3-itoral@igalia.com
State New
Headers show
Series "intel: propagate constants to first source of 2-src instructions" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga March 4, 2019, 12:15 p.m.
Even if it is not supported by the hardware, we will fix it up
in the combine constants pass.

v2:
 - This will enable new constant folding opportunities in the algebraic pass
   for MUL or ADD with types other than F, so do not assert on that type.
   For now we just skip anything that is not float and a later patch will
   expand the algebraic pass to support more constant folding scenarios.
---
 src/intel/compiler/brw_fs.cpp                 |  8 +--
 .../compiler/brw_fs_combine_constants.cpp     | 37 ++++++++++---
 .../compiler/brw_fs_copy_propagation.cpp      | 55 +++++++++----------
 3 files changed, 60 insertions(+), 40 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 2358acbeb59..bd588b55bde 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -2603,8 +2603,8 @@  fs_visitor::opt_algebraic()
             break;
          }
 
-         if (inst->src[0].file == IMM) {
-            assert(inst->src[0].type == BRW_REGISTER_TYPE_F);
+         if (inst->src[0].file == IMM &&
+             inst->src[0].type == BRW_REGISTER_TYPE_F) {
             inst->opcode = BRW_OPCODE_MOV;
             inst->src[0].f *= inst->src[1].f;
             inst->src[1] = reg_undef;
@@ -2616,8 +2616,8 @@  fs_visitor::opt_algebraic()
          if (inst->src[1].file != IMM)
             continue;
 
-         if (inst->src[0].file == IMM) {
-            assert(inst->src[0].type == BRW_REGISTER_TYPE_F);
+         if (inst->src[0].file == IMM &&
+             inst->src[0].type == BRW_REGISTER_TYPE_F) {
             inst->opcode = BRW_OPCODE_MOV;
             inst->src[0].f += inst->src[1].f;
             inst->src[1] = reg_undef;
diff --git a/src/intel/compiler/brw_fs_combine_constants.cpp b/src/intel/compiler/brw_fs_combine_constants.cpp
index a26eb67a00a..bb155caeccf 100644
--- a/src/intel/compiler/brw_fs_combine_constants.cpp
+++ b/src/intel/compiler/brw_fs_combine_constants.cpp
@@ -66,13 +66,31 @@  could_coissue(const struct gen_device_info *devinfo, const fs_inst *inst)
  * Returns true for instructions that don't support immediate sources.
  */
 static bool
-must_promote_imm(const struct gen_device_info *devinfo, const fs_inst *inst)
+must_promote_imm(const struct gen_device_info *devinfo,
+                 const fs_inst *inst, const int src_idx)
 {
    switch (inst->opcode) {
    case SHADER_OPCODE_INT_QUOTIENT:
    case SHADER_OPCODE_INT_REMAINDER:
    case SHADER_OPCODE_POW:
-      return devinfo->gen < 8;
+      return src_idx != 1 || devinfo->gen < 8;
+   case BRW_OPCODE_BFI1:
+   case BRW_OPCODE_ASR:
+   case BRW_OPCODE_SHL:
+   case BRW_OPCODE_SHR:
+   case BRW_OPCODE_SUBB:
+   case BRW_OPCODE_MACH:
+   case BRW_OPCODE_MUL:
+   case SHADER_OPCODE_MULH:
+   case BRW_OPCODE_ADD:
+   case BRW_OPCODE_OR:
+   case BRW_OPCODE_AND:
+   case BRW_OPCODE_XOR:
+   case BRW_OPCODE_ADDC:
+   case BRW_OPCODE_CMP:
+   case BRW_OPCODE_IF:
+   case BRW_OPCODE_SEL:
+      return src_idx != 1;
    case BRW_OPCODE_MAD:
    case BRW_OPCODE_LRP:
       return true;
@@ -335,13 +353,18 @@  fs_visitor::opt_combine_constants()
    foreach_block_and_inst(block, fs_inst, inst, cfg) {
       ip++;
 
-      if (!could_coissue(devinfo, inst) && !must_promote_imm(devinfo, inst))
+      const bool is_coissue_candidate = could_coissue(devinfo, inst);
+      if (!is_coissue_candidate && !must_promote_imm(devinfo, inst, -1))
          continue;
 
       for (int i = 0; i < inst->sources; i++) {
          if (inst->src[i].file != IMM)
             continue;
 
+         const bool must_promote = must_promote_imm(devinfo, inst, i);
+         if (!is_coissue_candidate && !must_promote)
+            continue;
+
          char data[8];
          brw_reg_type type;
          if (!get_constant_value(devinfo, inst, i, data, &type))
@@ -357,8 +380,8 @@  fs_visitor::opt_combine_constants()
                imm->inst = NULL;
             imm->block = intersection;
             imm->uses->push_tail(link(const_ctx, &inst->src[i]));
-            imm->uses_by_coissue += could_coissue(devinfo, inst);
-            imm->must_promote = imm->must_promote || must_promote_imm(devinfo, inst);
+            imm->uses_by_coissue += is_coissue_candidate;
+            imm->must_promote = imm->must_promote || must_promote;
             imm->last_use_ip = ip;
             if (type == BRW_REGISTER_TYPE_HF)
                imm->is_half_float = true;
@@ -371,8 +394,8 @@  fs_visitor::opt_combine_constants()
             memcpy(imm->bytes, data, size);
             imm->size = size;
             imm->is_half_float = type == BRW_REGISTER_TYPE_HF;
-            imm->uses_by_coissue = could_coissue(devinfo, inst);
-            imm->must_promote = must_promote_imm(devinfo, inst);
+            imm->uses_by_coissue = is_coissue_candidate;
+            imm->must_promote = must_promote;
             imm->first_use_ip = ip;
             imm->last_use_ip = ip;
          }
diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp
index c11b05b128a..33454e50861 100644
--- a/src/intel/compiler/brw_fs_copy_propagation.cpp
+++ b/src/intel/compiler/brw_fs_copy_propagation.cpp
@@ -624,10 +624,8 @@  fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
       case BRW_OPCODE_SHL:
       case BRW_OPCODE_SHR:
       case BRW_OPCODE_SUBB:
-         if (i == 1) {
-            inst->src[i] = val;
-            progress = true;
-         }
+         inst->src[i] = val;
+         progress = true;
          break;
 
       case BRW_OPCODE_MACH:
@@ -638,10 +636,7 @@  fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
       case BRW_OPCODE_AND:
       case BRW_OPCODE_XOR:
       case BRW_OPCODE_ADDC:
-         if (i == 1) {
-            inst->src[i] = val;
-            progress = true;
-         } else if (i == 0 && inst->src[1].file != IMM) {
+         if (i == 0 && inst->src[1].file != IMM) {
             /* Fit this constant in by commuting the operands.
              * Exception: we can't do this for 32-bit integer MUL/MACH
              * because it's asymmetric.
@@ -653,24 +648,25 @@  fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
              * Integer MUL with a non-accumulator destination will be lowered
              * by lower_integer_multiplication(), so don't restrict it.
              */
-            if (((inst->opcode == BRW_OPCODE_MUL &&
-                  inst->dst.is_accumulator()) ||
-                 inst->opcode == BRW_OPCODE_MACH) &&
-                (inst->src[1].type == BRW_REGISTER_TYPE_D ||
-                 inst->src[1].type == BRW_REGISTER_TYPE_UD))
+            if (((inst->opcode != BRW_OPCODE_MUL ||
+                  inst->dst.is_accumulator()) &&
+                 inst->opcode != BRW_OPCODE_MACH) ||
+                (inst->src[1].type != BRW_REGISTER_TYPE_D &&
+                 inst->src[1].type != BRW_REGISTER_TYPE_UD)) {
+               inst->src[0] = inst->src[1];
+               inst->src[1] = val;
+               progress = true;
                break;
-            inst->src[0] = inst->src[1];
-            inst->src[1] = val;
-            progress = true;
+            }
          }
+
+         inst->src[i] = val;
+         progress = true;
          break;
 
       case BRW_OPCODE_CMP:
       case BRW_OPCODE_IF:
-         if (i == 1) {
-            inst->src[i] = val;
-            progress = true;
-         } else if (i == 0 && inst->src[1].file != IMM) {
+         if (i == 0 && inst->src[1].file != IMM) {
             enum brw_conditional_mod new_cmod;
 
             new_cmod = brw_swap_cmod(inst->conditional_mod);
@@ -682,27 +678,28 @@  fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
                inst->src[1] = val;
                inst->conditional_mod = new_cmod;
                progress = true;
+               break;
             }
          }
+
+         inst->src[i] = val;
+         progress = true;
          break;
 
       case BRW_OPCODE_SEL:
-         if (i == 1) {
-            inst->src[i] = val;
-            progress = true;
-         } else if (i == 0 && inst->src[1].file != IMM) {
+         if (i == 0 && inst->src[1].file != IMM) {
             inst->src[0] = inst->src[1];
             inst->src[1] = val;
 
             /* If this was predicated, flipping operands means
              * we also need to flip the predicate.
              */
-            if (inst->conditional_mod == BRW_CONDITIONAL_NONE) {
-               inst->predicate_inverse =
-                  !inst->predicate_inverse;
-            }
-            progress = true;
+            if (inst->conditional_mod == BRW_CONDITIONAL_NONE)
+               inst->predicate_inverse = !inst->predicate_inverse;
+         } else {
+            inst->src[i] = val;
          }
+         progress = true;
          break;
 
       case SHADER_OPCODE_UNTYPED_ATOMIC: