On Sat, Oct 6, 2018 at 1:24 AM Ilia Mirkin <imir...@alum.mit.edu> wrote:
>
> This happens in situations where we might do
>
> vec.wzyx[i] = ...
>
> The swizzle would get effectively ignored because of the interaction
> between how ir_assignment->set_lhs works and overwriting the write_mask.
> There are two cases, one where i is a constant, and another where i is
> variable. We have to be extra-careful in both cases.
>
> Fixes the following WebGL test:
>
>
> https://www.khronos.org/registry/webgl/sdk/tests/conformance2/glsl3/vector-dynamic-indexing-swizzled-lvalue.html
>
> And the new piglit tests:
>
> swizzled-writemask-indexing-nonconst.shader_test
> swizzled-writemask-indexing.shader_test
>
> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>
> v1 -> v2:
> - include info about piglit tests
> - add a fix for the nonconst case -- doing set_lhs last instead of
> first.
>
> Ian Romanick reviewed v1, but given the subtlety of the additional
> change for v2, going to leave that off, prefering another review.
>
> No regressions running this through Intel CI.
>
> src/compiler/glsl/lower_vector_derefs.cpp | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/lower_vector_derefs.cpp
> b/src/compiler/glsl/lower_vector_derefs.cpp
> index 7583d1fdd3e..223e420550b 100644
> --- a/src/compiler/glsl/lower_vector_derefs.cpp
> +++ b/src/compiler/glsl/lower_vector_derefs.cpp
> @@ -59,8 +59,7 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
> if (!deref->array->type->is_vector())
> return ir_rvalue_enter_visitor::visit_enter(ir);
>
> - ir_dereference *const new_lhs = (ir_dereference *) deref->array;
> - ir->set_lhs(new_lhs);
> + ir_rvalue *const new_lhs = deref->array;
>
> void *mem_ctx = ralloc_parent(ir);
> ir_constant *old_index_constant =
> @@ -72,8 +71,15 @@ vector_deref_visitor::visit_enter(ir_assignment *ir)
> ir->rhs,
> deref->array_index);
> ir->write_mask = (1 << new_lhs->type->vector_elements) - 1;
> + ir->set_lhs(new_lhs);
> + } else if (new_lhs->ir_type != ir_type_swizzle) {
> + ir->set_lhs(new_lhs);
> + ir->write_mask = 1 << old_index_constant->get_uint_component(0);
> } else {
> - ir->write_mask = 1 << old_index_constant->get_int_component(0);
> + // If there "new" lhs is a swizzle, use the set_lhs helper to instead
> + // swizzle the rhs.
Urgh, forgot to fix this based on Ian's feedback last time. This now reads
/* If there "new" LHS is a swizzle, use the set_lhs helper to instead
* swizzle the RHS.
*/
locally.
> + unsigned component[1] = { old_index_constant->get_uint_component(0) };
> + ir->set_lhs(new(mem_ctx) ir_swizzle(new_lhs, component, 1));
> }
>
> return ir_rvalue_enter_visitor::visit_enter(ir);
> --
> 2.16.4
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
