[Mesa-dev] [PATCH 1/2] i965/fs: fix copy/constant propagation regioning checks

[Iago Toral Quiroga]

We were not accounting for reg_suboffset in the check for the start
of the region. Also, fs_reg::regs_read() already takes the stride
into account, so we should not multiply its result by the stride
again.

Incidentally, the stride fix opens more possibilities for copy-propagation
which uncovered another bug where we can copy-propagate an entry that has
been partially invalidated (only fp64 code seems affected by this according
to piglit). The next patch will fix this.
---
 .../drivers/dri/i965/brw_fs_copy_propagation.cpp   | 23 ++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
index 591db53..3e89a0d 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -329,6 +329,17 @@ can_take_stride(fs_inst *inst, unsigned arg, unsigned stride,
    return true;
 }
 
+static bool
+region_match(fs_reg src, unsigned regs_read,
+             fs_reg dst, unsigned regs_written)
+{
+   unsigned src_start = src.reg_offset * 32 + src.subreg_offset;
+   unsigned dst_start = dst.reg_offset * 32 + dst.subreg_offset;
+   unsigned src_end = src_start + regs_read * 32;
+   unsigned dst_end = dst_start + regs_written * 32;
+   return src_start >= dst_start && src_end <= dst_end;
+}
+
 bool
 fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
 {
@@ -351,10 +362,8 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
    /* Bail if inst is reading a range that isn't contained in the range
     * that entry is writing.
     */
-   if (inst->src[arg].reg_offset < entry->dst.reg_offset ||
-       (inst->src[arg].reg_offset * 32 + inst->src[arg].subreg_offset +
-        inst->regs_read(arg) * inst->src[arg].stride * 32) >
-       (entry->dst.reg_offset + entry->regs_written) * 32)
+   if (!region_match(inst->src[arg], inst->regs_read(arg),
+                     entry->dst, entry->regs_written))
       return false;
 
    /* we can't generally copy-propagate UD negations because we
@@ -534,10 +543,8 @@ fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
       /* Bail if inst is reading a range that isn't contained in the range
        * that entry is writing.
        */
-      if (inst->src[i].reg_offset < entry->dst.reg_offset ||
-          (inst->src[i].reg_offset * 32 + inst->src[i].subreg_offset +
-           inst->regs_read(i) * inst->src[i].stride * 32) >
-          (entry->dst.reg_offset + entry->regs_written) * 32)
+      if (!region_match(inst->src[i], inst->regs_read(i),
+                        entry->dst, entry->regs_written))
          continue;
 
       fs_reg val = entry->src;
-- 
2.1.4