[Mesa-dev] [PATCH] i965: vs optimization fix: Check val.negate in accumulator_contains()

[Eric Anholt]

On Sat, 23 Jul 2011 09:38:05 -0700, Paul Berry <stereotype441 at gmail.com> wrote:
> On 22 July 2011 18:17, Eric Anholt <eric at anholt.net> wrote:
> > On Fri, 22 Jul 2011 15:07:57 -0700, Paul Berry <stereotype441 at gmail.com> wrote:
> >> When emitting a MAC instruction in a vertex shader, brw_vs_emit()
> >> calls accumulator_contains() to determine whether the accumulator
> >> already contains the appropriate addend; if it does, then we can avoid
> >> emitting an unnecessary MOV instruction.
> >>
> >> However, accumulator_contains() wasn't checking the val.negate flag.
> >> As a result, if the accumulator contained the negation of the desired
> >> value, we would generate an incorrect shader.
> >>
> >> Fixes piglit test vs-refract-vec4-vec4-float.
> >>
> >> Tested on Gen5 and Gen6.
> >
> > Looks like val.abs doesn't currently need this treatement because we
> > never generate regs with it where they'd appear as the arg to MAD.
> 
> Oh, thanks for alerting me to the existence of val.abs.  I believe
> your analysis is correct, but if it's all the same to you I think I'd
> rather switch to:
> 
> if (val.negate || val.abs)
>     return GL_FALSE;
> 
> Since the efficiency cost of checking val.abs is negligible, and it
> takes less thinking to be certain that this is correct.

Sounds good.
-------------- 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/20110724/9388868b/attachment.pgp>