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

Paul Berry stereotype441 at gmail.com
Sat Jul 23 09:38:05 PDT 2011


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.


More information about the mesa-dev mailing list