[Mesa-dev] [PATCH] glsl: Add ir_binop_vector_extract in NIR

[Iago Toral]

On Fri, 2018-06-01 at 12:26 +0200, Iago Toral wrote:
> On Fri, 2018-06-01 at 11:25 +0200, Juan A. Suarez Romero wrote:
> > On Wed, 2018-05-30 at 15:10 -0700, Eric Anholt wrote:
> > > "Juan A. Suarez Romero" <jasuarez at igalia.com> writes:
> > > 
> > > > Implement ir_binop_vector_extract using NIR operations. Based
> > > > on
> > > > SPIR-V
> > > > to NIR approach.
> > > > 
> > > > This fixes:
> > > > dEQP-
> > > > GLES3.functional.shaders.indexing.moredynamic.with_value_from_i
> > > > nd
> > > > exing_expression_fragment
> > > > Piglit's glsl-fs-vec4-indexing-8.shader_test
> > > > 
> > > > Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> > > > ---
> > > > 
> > > > Pending to verify that this also fixes https://bugs.freedesktop
> > > > .o
> > > > rg/show_bug.cgi?id=105438
> > > > 
> > > >  src/compiler/glsl/glsl_to_nir.cpp | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > > 
> > > > diff --git a/src/compiler/glsl/glsl_to_nir.cpp
> > > > b/src/compiler/glsl/glsl_to_nir.cpp
> > > > index 8e5e9c34912..5fc420d856f 100644
> > > > --- a/src/compiler/glsl/glsl_to_nir.cpp
> > > > +++ b/src/compiler/glsl/glsl_to_nir.cpp
> > > > @@ -1928,6 +1928,17 @@ nir_visitor::visit(ir_expression *ir)
> > > >              unreachable("not reached");
> > > >        }
> > > >        break;
> > > > +   case ir_binop_vector_extract: {
> > > > +      unsigned swiz[4] = { 0 };
> > > > +      result = nir_swizzle(&b, srcs[0], swiz, 1, true);
> > > > +      for (unsigned i = 1; i < ir->operands[0]->type-
> > > > > vector_elements; i++) {
> > > > 
> > > > +         swiz[0] = i;
> > > > +         nir_ssa_def *swizzled = nir_swizzle(&b, srcs[0],
> > > > swiz,
> > > > 1, true);
> > > 
> > > You could use nir_channel(&b, srcs[0], i) here and above to
> > > simplify
> > > (which I think gets the use_fmov argument right, as well).  Other
> > > than
> > > that,
> > > 
> > 
> > I'm checking nir_channel(), and it sets use_fmov to false, rather
> > than true.
> 
> I don't think that is a problem, right? does that make the test fail?

I just checked and it seems to work just fine, as expected, feel free
to add my Rb as well to the patch with Eric's suggestions.

Iago

> 
> > 
> > 	J.A.
> > 
> > > Reviewed-by: Eric Anholt <eric at anholt.net>
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev