[Mesa-dev] Incorrect assertion in ir_assignment?

Iago Toral itoral at igalia.com
Thu Feb 12 23:19:30 PST 2015


On Thu, 2015-02-12 at 08:05 -0800, Ian Romanick wrote:
> 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)

Thanks for explaining this Ian. This fixes the problem I mentioned in
the texture gradient lowering pass. I'll send a patch shortly.

Iago

> 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
> 
> 




More information about the mesa-dev mailing list