Mesa (master): intel/compiler: Relax some conditions in try_copy_propagate

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: 52eb47c8d4840aa4fa74e869beccf29e812d7d65
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=52eb47c8d4840aa4fa74e869beccf29e812d7d65

Author: Ian Romanick <ian.d.romanick at intel.com>
Date:   Sun Dec  6 16:26:04 2020 -0800

intel/compiler: Relax some conditions in try_copy_propagate

Previously can_do_source_mods was used to determine whether a value with
a source modifier or a value from a scalar source (e.g., a uniform)
could be copy propagated.  The former is a superset of the latter, so
this always produces correct results, but it is overly restrictive.  For
example, a BFI instruction can't have source modifiers, but it can have
scalar sources.

This was originally authored to prevent a small number of shader-db
regressions in a commit that marked SHR has not being able to have
source modifiers.  That commit has since been dropped in favor of a
different method.

v2: Refactor register region restriction detection to a helper function.
Suggested by Jason.

No fossil-db changes on any Intel platform.

All Gen7+ platforms had similar results. (Ice Lake shown)
total instructions in shared programs: 20039111 -> 20038943 (<.01%)
instructions in affected programs: 31736 -> 31568 (-0.53%)
helped: 104
HURT: 0
helped stats (abs) min: 1 max: 9 x̄: 1.62 x̃: 1
helped stats (rel) min: 0.30% max: 0.88% x̄: 0.45% x̃: 0.42%
95% mean confidence interval for instructions value: -2.03 -1.20
95% mean confidence interval for instructions %-change: -0.47% -0.42%
Instructions are helped.

total cycles in shared programs: 980309750 -> 980308897 (<.01%)
cycles in affected programs: 591078 -> 590225 (-0.14%)
helped: 70
HURT: 26
helped stats (abs) min: 2 max: 622 x̄: 23.94 x̃: 4
helped stats (rel) min: <.01% max: 2.85% x̄: 0.33% x̃: 0.12%
HURT stats (abs)   min: 2 max: 520 x̄: 31.65 x̃: 6
HURT stats (rel)   min: 0.02% max: 2.45% x̄: 0.34% x̃: 0.15%
95% mean confidence interval for cycles value: -26.41 8.64
95% mean confidence interval for cycles %-change: -0.27% -0.03%
Inconclusive result (value mean confidence interval includes 0).

No shader-db changes on earlier Intel platforms.

Reviewed-by: Anuj Phogat anuj.phogat at gmail.com [v1]
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9237>

---

 src/intel/compiler/brw_fs_copy_propagation.cpp   | 12 +++++++++---
 src/intel/compiler/brw_ir.h                      |  6 ++++++
 src/intel/compiler/brw_shader.cpp                | 13 +++++++++++++
 src/intel/compiler/brw_vec4_copy_propagation.cpp | 11 +++++++++--
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp
index 6896987055f..343bac23889 100644
--- a/src/intel/compiler/brw_fs_copy_propagation.cpp
+++ b/src/intel/compiler/brw_fs_copy_propagation.cpp
@@ -510,11 +510,17 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
 
    bool has_source_modifiers = entry->src.abs || entry->src.negate;
 
-   if ((has_source_modifiers || entry->src.file == UNIFORM ||
-        !entry->src.is_contiguous()) &&
-       !inst->can_do_source_mods(devinfo))
+   if (has_source_modifiers && !inst->can_do_source_mods(devinfo))
       return false;
 
+   /* Reject cases that would violate register regioning restrictions. */
+   if ((entry->src.file == UNIFORM || !entry->src.is_contiguous()) &&
+       ((devinfo->gen == 6 && inst->is_math()) ||
+        inst->is_send_from_grf() ||
+        inst->uses_indirect_addressing())) {
+      return false;
+   }
+
    if (has_source_modifiers &&
        inst->opcode == SHADER_OPCODE_GEN4_SCRATCH_WRITE)
       return false;
diff --git a/src/intel/compiler/brw_ir.h b/src/intel/compiler/brw_ir.h
index 01f81feddb4..a98eab9f8fb 100644
--- a/src/intel/compiler/brw_ir.h
+++ b/src/intel/compiler/brw_ir.h
@@ -101,6 +101,12 @@ struct backend_instruction : public exec_node {
    bool reads_accumulator_implicitly() const;
    bool writes_accumulator_implicitly(const struct gen_device_info *devinfo) const;
 
+   /**
+    * Instructions that use indirect addressing have additional register
+    * regioning restrictions.
+    */
+   bool uses_indirect_addressing() const;
+
    void remove(bblock_t *block);
    void insert_after(bblock_t *block, backend_instruction *inst);
    void insert_before(bblock_t *block, backend_instruction *inst);
diff --git a/src/intel/compiler/brw_shader.cpp b/src/intel/compiler/brw_shader.cpp
index f52295fa310..cba0b589dde 100644
--- a/src/intel/compiler/brw_shader.cpp
+++ b/src/intel/compiler/brw_shader.cpp
@@ -931,6 +931,19 @@ backend_instruction::is_control_flow() const
    }
 }
 
+bool
+backend_instruction::uses_indirect_addressing() const
+{
+   switch (opcode) {
+   case SHADER_OPCODE_BROADCAST:
+   case SHADER_OPCODE_CLUSTER_BROADCAST:
+   case SHADER_OPCODE_MOV_INDIRECT:
+      return true;
+   default:
+      return false;
+   }
+}
+
 bool
 backend_instruction::can_do_source_mods() const
 {
diff --git a/src/intel/compiler/brw_vec4_copy_propagation.cpp b/src/intel/compiler/brw_vec4_copy_propagation.cpp
index 9e4637e9753..a0df115d4a3 100644
--- a/src/intel/compiler/brw_vec4_copy_propagation.cpp
+++ b/src/intel/compiler/brw_vec4_copy_propagation.cpp
@@ -349,10 +349,17 @@ try_copy_propagate(const struct gen_device_info *devinfo,
    /* gen6 math and gen7+ SENDs from GRFs ignore source modifiers on
     * instructions.
     */
-   if ((has_source_modifiers || value.file == UNIFORM ||
-        value.swizzle != BRW_SWIZZLE_XYZW) && !inst->can_do_source_mods(devinfo))
+   if (has_source_modifiers && !inst->can_do_source_mods(devinfo))
       return false;
 
+   /* Reject cases that would violate register regioning restrictions. */
+   if ((value.file == UNIFORM || value.swizzle != BRW_SWIZZLE_XYZW) &&
+       ((devinfo->gen == 6 && inst->is_math()) ||
+        inst->is_send_from_grf() ||
+        inst->uses_indirect_addressing())) {
+      return false;
+   }
+
    if (has_source_modifiers &&
        value.type != inst->src[arg].type &&
        !inst->can_change_types())