r600/sb: Check whether optimizations would result in reladdr conflict

v2: * Check whether the node src and dst registers are NULL before using
      them.
    * fix a type in the commit message.

Two cases are handled with this patch:

1. If copy propagation tries to eliminated a move from a relative
   array access then it could optimize

     MOV R1, ARRAY[RELADDR_1]
     MOV R2, ARRAY[RELADDR_2]
     OP2 R3, R1 R2

   into

     OP2 R3, ARRAY[RELADDR_1], ARRAY[RELADDR_2]

   which is forbidden, because there is only one address register available.

2. When MULADD(x,a,MUL(x,c)) is handled

      MUL TMP, R1, ARRAY[RELADDR_1]
      MULLADD R3, R1, ARRAY[RELADDR_2], TMP

   by folding this into

      ADD TMP, ARRAY[RELADDR_2], ARRAY[RELADDR_1]
      MUL R3, R1, TMP

   which is also forbidden.

Test for these cases and reject the optimization if a forbidden combination
of relative access would be created.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103142
Signed-off-by: Gert Wollny <gw.fossdev@gmail.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>

diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp b/src/gallium/drivers/r600/sb/sb_expr.cpp
index 7d43ef1d1d588cff8d38d172fb9ec78b04866026..1df78da6608343cc0cf32a7642b4b4ed0b8483c1 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_expr.cpp
+++ b/src/gallium/drivers/r600/sb/sb_expr.cpp
@@ -412,7 +412,8 @@ bool expr_handler::fold_alu_op1(alu_node& n) {
                                n.bc.op == ALU_OP1_MOVA_GPR_INT)
                                && n.bc.clamp == 0 && n.bc.omod == 0
                                && n.bc.src[0].abs == 0 && n.bc.src[0].neg == 0 &&
-                               n.src.size() == 1 /* RIM/SIM can be appended as additional values */) {
+                               n.src.size() == 1 /* RIM/SIM can be appended as additional values */
+                               && n.dst[0]->no_reladdr_conflict_with(v0)) {
                        assign_source(n.dst[0], v0);
                        return true;
                }
@@ -1027,9 +1028,17 @@ bool expr_handler::fold_alu_op3(alu_node& n) {
                                es1 = 1;
                        }
 
-                       if (es0 != -1) {
-                               value *va0 = es0 == 0 ? v1 : v0;
-                               value *va1 = es1 == 0 ? mv1 : mv0;
+                       value *va0 = es0 == 0 ? v1 : v0;
+                       value *va1 = es1 == 0 ? mv1 : mv0;
+
+                       /* Don't fold if no equal multipliers were found.
+                        * Also don#t fold if the operands of the to be created ADD are both
+                        * relatively accessed with different AR values because that would
+                        * create impossible code.
+                        */
+                       if (es0 != -1 &&
+                           (!va0->is_rel() || !va1->is_rel() ||
+                            (va0->rel == va1->rel))) {
 
                                alu_node *add = sh.create_alu();
                                add->bc.set_op(ALU_OP2_ADD);

diff --git a/src/gallium/drivers/r600/sb/sb_ir.h b/src/gallium/drivers/r600/sb/sb_ir.h
index 245af961e6996a3eba7bb2f7548137e3c73bca28..c7a94fcb930ecd9d95703b6fcbcfd969a1e842b7 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_ir.h
+++ b/src/gallium/drivers/r600/sb/sb_ir.h
@@ -616,6 +616,12 @@ public:
                }
        }
 
+       /* Check whether copy-propagation of src into this would create an access
+        * conflict with relative addressing, i.e. an operation that tries to access
+        * array elements with different address register values.
+        */
+       bool no_reladdr_conflict_with(value *src);
+
        val_set interferences;
        unsigned uid;
 };

diff --git a/src/gallium/drivers/r600/sb/sb_valtable.cpp b/src/gallium/drivers/r600/sb/sb_valtable.cpp
index dd336bc41772eca25cb62b88e1072f29e3936fab..f847138cac44e38582ffb6266e5c6314f0b1e45f 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_valtable.cpp
+++ b/src/gallium/drivers/r600/sb/sb_valtable.cpp
@@ -296,6 +296,42 @@ void value::delete_uses() {
        uses.erase(uses.begin(), uses.end());
 }
 
+bool value::no_reladdr_conflict_with(value *src)
+{
+       /*  if the register is not relative, it can't create an relative access conflict */
+       if (!src->is_rel())
+               return true;
+
+       /* If the destination is AR then we accept the copy propagation, because the
+        * scheduler actually re-creates the address loading operation and will
+        * signal interference if there is an address register load and it will fail
+        * because of this.
+        */
+       if (gvalue()->is_AR())
+               return true;
+
+       /* For all nodes that use this value test whether the operation uses another
+        * relative access with a different address value. If found, signal conflict.
+        */
+       for (uselist::const_iterator N = uses.begin(); N != uses.end(); ++N) {
+               for (vvec::const_iterator V = (*N)->src.begin(); V != (*N)->src.end(); ++V) {
+                       if (*V) {
+                               value *v = (*V)->gvalue();
+                               if (v != src && v->is_rel() && v->rel != src->rel)
+                                       return false;
+                       }
+               }
+               for (vvec::const_iterator V = (*N)->dst.begin(); V != (*N)->dst.end(); ++V) {
+                       if (*V) {
+                               value *v = (*V)->gvalue();
+                               if (v && v != src && v->is_rel() && (v->rel != src->rel))
+                                       return false;
+                       }
+               }
+       }
+       return true;
+}
+
 void ra_constraint::update_values() {
        for (vvec::iterator I = values.begin(), E = values.end(); I != E; ++I) {
                assert(!(*I)->constraint);