[Mesa-dev] Incorrect assertion in ir_assignment?

[Ian Romanick]

On 02/12/2015 06:19 AM, Iago Toral wrote:
> Hi,
> 
> is this assertion in ir_assignment::ir_assignment() src/glsl/ir.cpp
> correct?
> 
> assert(lhs_components == this->rhs->type->vector_elements);
> 
> We will hit this any time we emit code such as this:
> 
> assign(var1, expr, WRITEMASK_XY)

In this case expr must have two components.  I think you want

  assign(var1, swizzle_for_size(expr, 2), WRITEMASK_XY)

GLSL IR assignments match the components in order.  If var1 is a vec3,
all of the following are valid and different:

  assign(var1, swizzle_for_size(expr, 2), WRITEMASK_XY)

  assign(var1, swizzle_for_size(expr, 2), WRITEMASK_XZ)

  assign(var1, swizzle_for_size(expr, 2), WRITEMASK_YZ)

So the writemask is definitely not useless.

> where expr and var1 are vec3. At least if we emit that code from
> lowering passes inside the driver. There is an example of this situation
> in the i965 code (see brw_lower_texture_gradients.cpp).
> 
> It feels like that assertion makes it so that the writemask parameter is
> useless, since we always have to write all the elements available in the
> rhs... I was tempted to rewrite that assertion as:
> 
> assert(lhs_components <= this->rhs->type->vector_elements);
> 
> or even remove it completely (just comparing the element count and not
> the channels we are writing to does not seem to give us much...), but
> the commit that introduced the assertion was willing to force that
> behavior for some reason.
> 
> If we want to keep the assertion then we have to rewrite the code in
> brw_lower_texture_gradients.cpp (that is trivial because we don't really
> need to restrict the offending assignment to XY only, I already have a
> patch for that), but I wonder if that is really what we want to do here.
> 
> Any thoughts?
> 
> Iago
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev