Discussion:
[PATCH] r600/sb: do not convert if-blocks that contain indirect array access
Add Reply
Gert Wollny
2017-12-06 16:42:02 UTC
Reply
Permalink
Raw Message
If an array is accessed within an if block, then currently it is not known
whether the value in the address register is involved in the evaluation of the
if condition, and converting the if condition may actually result in
out-of-bounds array access. Consequently, if blocks that contain indirect array
access should not be converted.

Fixes piglits on r600/BARTS:
spec/glsl-1.10/execution/variable-indexing/
vs-output-array-float-index-wr
vs-output-array-vec3-index-wr
vs-output-array-vec4-index-wr

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104143

Signed-off-by: Gert Wollny <***@gmail.com>
---
* A better fix would probably contain some tracking to see whether the address
register value derives from a value used in the if condition, but without
doing some major refactoring and bringing the r600/sb code under test I'm kind
of afraid to touch it.
* I don't have mesa-git write access.

Best,
Gert

src/gallium/drivers/r600/sb/sb_if_conversion.cpp | 2 +-
src/gallium/drivers/r600/sb/sb_ir.cpp | 2 ++
src/gallium/drivers/r600/sb/sb_ir.h | 3 ++-
3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/r600/sb/sb_if_conversion.cpp b/src/gallium/drivers/r600/sb/sb_if_conversion.cpp
index 3f2b1b1b92..3f6431b80f 100644
--- a/src/gallium/drivers/r600/sb/sb_if_conversion.cpp
+++ b/src/gallium/drivers/r600/sb/sb_if_conversion.cpp
@@ -136,7 +136,7 @@ bool if_conversion::check_and_convert(region_node *r) {
);

if (s.region_count || s.fetch_count || s.alu_kill_count ||
- s.if_count != 1 || s.repeat_count)
+ s.if_count != 1 || s.repeat_count || s.uses_ar)
return false;

unsigned real_alu_count = s.alu_count - s.alu_copy_mov_count;
diff --git a/src/gallium/drivers/r600/sb/sb_ir.cpp b/src/gallium/drivers/r600/sb/sb_ir.cpp
index d989dce62c..6e44193c1e 100644
--- a/src/gallium/drivers/r600/sb/sb_ir.cpp
+++ b/src/gallium/drivers/r600/sb/sb_ir.cpp
@@ -461,6 +461,8 @@ void container_node::collect_stats(node_stats& s) {
++s.alu_kill_count;
else if (a->is_copy_mov())
++s.alu_copy_mov_count;
+ if (a->uses_ar())
+ s.uses_ar = true;
} else if (n->is_fetch_inst())
++s.fetch_count;
else if (n->is_cf_inst())
diff --git a/src/gallium/drivers/r600/sb/sb_ir.h b/src/gallium/drivers/r600/sb/sb_ir.h
index ec973e7bfc..67c7cd8aa4 100644
--- a/src/gallium/drivers/r600/sb/sb_ir.h
+++ b/src/gallium/drivers/r600/sb/sb_ir.h
@@ -726,11 +726,12 @@ struct node_stats {
unsigned depart_count;
unsigned repeat_count;
unsigned if_count;
+ bool uses_ar;

node_stats() : alu_count(), alu_kill_count(), alu_copy_mov_count(),
cf_count(), fetch_count(), region_count(),
loop_count(), phi_count(), loop_phi_count(), depart_count(),
- repeat_count(), if_count() {}
+ repeat_count(), if_count(), uses_ar(false) {}

void dump();
};
--
2.13.6
Loading...