On 08/21/2013 05:57 PM, Dominik Behr wrote:
Fixes a bug where if an uniform array is passed to a function the accesses
to the array are not propagated so later all but the first vector of the
uniform array are removed in parcel_out_uniform_storage resulting in
broken shaders and out of bounds access to arrays in
brw::vec4_visitor::pack_uniform_registers.
Signed-off-by: Dominik Behr <db...@chromium.org>
---
src/glsl/link_functions.cpp | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/src/glsl/link_functions.cpp b/src/glsl/link_functions.cpp
index 6b3e154..32e2012 100644
--- a/src/glsl/link_functions.cpp
+++ b/src/glsl/link_functions.cpp
@@ -173,6 +173,32 @@ public:
return visit_continue;
}
+ virtual ir_visitor_status visit_leave(ir_call *ir)
+ {
+ /* traverse list of function parameters, and for array parameters
+ propagate max_array_access, do it when leaving the node
+ so the childen would propagate their array accesses first */
children
I also think this comment should be expanded. On the first read of this
code, it wasn't obvious to me why this was necessary. It's worth
mentioning that the important case is when the reference to the array at
the call site does not reference a specific element. In that case the
access range of the from the callee is propagated back to the array
passed in.
+ exec_list_iterator sig_param_iter = ir->callee->parameters.iterator();
+ exec_list_iterator param_iter = ir->actual_parameters.iterator();
While iterators are a fine design pattern, the implementation of them in
the compiler front-end is garbage (and I made that implementation). We
stopped writing new code to use the iterators years ago. There is other
code that does a similar double-walk of the signature parameters and
actual parameters lists.
I think the best example is in
lower_clip_distance_visitor::visit_leave(ir_call *ir) in
lower_clip_distance.cpp.
+ while (param_iter.has_next()) {
+ ir_variable *sig_param = (ir_variable *) sig_param_iter.get();
+ ir_rvalue *param = (ir_rvalue *) param_iter.get();
+ assert(sig_param);
+ assert(param);
+ if (sig_param->type->is_array()) {
+ ir_dereference_variable *deref = param->as_dereference_variable();
+ if (deref && deref->var && deref->var->type->is_array()) {
+ if (sig_param->max_array_access > deref->var->max_array_access)
{
+ deref->var->max_array_access = sig_param->max_array_access;
+ }
This should end up looking more similar to the code in
call_link_visitor::visit(ir_dereference_variable *ir) around line 200
(without your patch).
var->max_array_access =
MAX2(var->max_array_access, ir->var->max_array_access);
+ }
+ }
+ sig_param_iter.next();
+ param_iter.next();
+ }
+ return visit_continue;
+ }
+
virtual ir_visitor_status visit(ir_dereference_variable *ir)
{
if (hash_table_find(locals, ir->var) == NULL) {
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
