[Mesa-dev] [PATCH] i965/vs: Fix bogus assertion in emit_block_move()

[Eric Anholt]

On Thu, 19 Jan 2012 18:28:50 -0800, Paul Berry <stereotype441 at gmail.com> wrote:
> i965 processes assignments of whole structures using
> vec4_visitor::emit_block_move, a recursive function which visits each
> element of a structure or array (to arbitrary nesting depth) and
> copies it from the source to the destination.  Then it increments the
> source and destination register numbers so that further recursive
> invocations will copy the rest of the structure.  In addition, it sets
> the swizzle field for the source register to an appropriate value of
> swizzle_for_size(...) for the size of each element being copied, so
> that later optimization passes won't be fooled into thinking that
> unused vector elements are live.
> 
> This all works fine.  However, emit_block_move also contains an
> assertion to verify, before setting the swizzle field for the source
> register, that the source register doesn't already contain a
> nontrivial swizzle.  The intention is to make sure that the caller of
> emit_block_move hasn't already done some swizzling of the data before
> the call, which emit_block_move would then counteract when it
> overwrites the swizzle field.  But the assertion is at the lowest
> level of nesting of emit_block_move, which means that after the first
> element is copied, instead of checking the swizzle field set by the
> caller, it checks the swizzle field used when moving the previous
> element.  That means that if the structure contains elements of
> different vector sizes (which therefore require different swizzles),
> the assertion will erroneously fire.
> 
> This patch moves the assertion from emit_block_move to the calling
> function, vec4_visitor::visit(ir_assignment *).  Since the caller is
> non-recursive, the assertion will only happen once, and won't be
> fooled by emit_block_move's modification of the swizzle field.
> 
> This patch also reverts commit fe006a7 (i965/vs: Fix swizzle related
> assertion), which attempted to fix the bug by making the assertion
> more lenient, but only worked properly for structures, arrays, and
> matrices in which each constituent vector is the same size.
> 
> This fixes the problem described in comment 9 of
> https://bugs.freedesktop.org/show_bug.cgi?id=40865.  Unfortunately, it
> doesn't fix the whole bug, since the test in question is also failing
> due to lack of register spilling support in the VS.

Sigh at multiple bugs reported in a single bug report.

Still,

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120120/3312dd85/attachment.pgp>