On Tuesday, November 18, 2014 09:15:06 PM Chris Forbes wrote: > When converting a uniform array reference to a pull constant load, the > `reladdr` expression itself may have its own `reladdr`, arbitrarily > deeply. This arises from expressions like: > > a[b[x]] where a, b are uniform arrays (or lowered const arrays), > and x is not a constant. > > Just iterate the lowering to pull constants until we stop seeing these > nested. For most shaders, there will be only one pass through this loop. > > Fixes the piglit test: > tests/spec/glsl-1.20/linker/double-indirect-1.shader_test > > Signed-off-by: Chris Forbes <chr...@ijw.co.nz> > Cc: "10.3 10.4" <mesa-sta...@lists.freedesktop.org> > --- > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 66 +++++++++++++++----------- > 1 file changed, 37 insertions(+), 29 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > index af7ca0c..22a6fb9 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > @@ -3354,6 +3354,7 @@ vec4_visitor::move_uniform_array_access_to_pull_constants() > { > int pull_constant_loc[this->uniforms]; > memset(pull_constant_loc, -1, sizeof(pull_constant_loc)); > + bool nested_reladdr; > > /* Walk through and find array access of uniforms. Put a copy of that > * uniform in the pull constant buffer. > @@ -3361,44 +3362,51 @@ vec4_visitor::move_uniform_array_access_to_pull_constants() > * Note that we don't move constant-indexed accesses to arrays. No > * testing has been done of the performance impact of this choice. > */ > - foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) { > - for (int i = 0 ; i < 3; i++) { > - if (inst->src[i].file != UNIFORM || !inst->src[i].reladdr) > - continue; > + do { > + nested_reladdr = false;
Yikes. Good find on the bug. Wrapping it in a loop like this is definitely nice and simple, which is great for stable branches. It might be worth adding a TODO comment for doing it more efficiently someday: /* TODO: We could refactor this and avoid doing N passes over the instruction * stream (where N is the reladdr nesting depth). */ I think this is fine, though, since the reladdr depth is probably 0 or 1...and we just found our first case of 2 today... I think you need an equivalent patch for the FS. As is, Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev