summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGert Wollny <[email protected]>2018-02-08 15:11:58 +0100
committerDave Airlie <[email protected]>2018-02-09 10:00:38 +1000
commitc36172e387b68aed083bb751d48733919f59bef7 (patch)
tree61a8eb008a00a46f762ee4fdfd52e47f79a5e69b
parent1d871aa6268159cdc63ef846599456d9ba567206 (diff)
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 <[email protected]> Reviewed-by: Dave Airlie <[email protected]>
-rw-r--r--src/gallium/drivers/r600/sb/sb_expr.cpp17
-rw-r--r--src/gallium/drivers/r600/sb/sb_ir.h6
-rw-r--r--src/gallium/drivers/r600/sb/sb_valtable.cpp36
3 files changed, 55 insertions, 4 deletions
diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp b/src/gallium/drivers/r600/sb/sb_expr.cpp
index 7d43ef1d1d5..1df78da6608 100644
--- 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 245af961e69..c7a94fcb930 100644
--- 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 dd336bc4177..f847138cac4 100644
--- 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);