[Mesa-dev] [PATCH 2/3] i965/fs: Fix comparisions with uint negation.

[Eric Anholt]

On Tue, 04 Oct 2011 23:55:21 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 10/03/2011 03:41 PM, Eric Anholt wrote:
> > The condmod instruction ends up generating garbage condition codes,
> > because apparently the comparison happens on the accumulator value (33
> > bits for UD), not the truncated value that would be written.
> > 
> > Fixes fs-op-neg-*
> 
> I am uneasy about these two patches for a number of reasons.
> 
> 1. fs-op-neg-* and vs-op-neg-* fail on AMD (Catalyst 11.7),
>    which makes me wonder if the tests are correct.
> 
> AMD does the expected behavior if you change the line:
>   uint result = (- arg0);
> to this:
>   uint result = uint(-int(arg0));
> or this:
>   uint result = 0 - arg0;
> 
> While the GLSL spec isn't terribly clear about how unary negate works on
> unsigned, the fact -arg0 != 0 - arg0 seems horrendously broken.  So
> maybe AMD's driver is just buggy.

Yeah, that seems just plain broken, based on the "0 - arg0" result.

> 2. I can't seem to find documentation to explain this.
> 
> Ivybridge uses explicit CMP instructions rather than the embedded
> conditional modifier on the IF instruction, and these tests still fail
> there.  So CMP must be using the accumulator as well?  But I see no
> documentation that actually states anything of the sort.

I think it's using the
result-that-goes-to-the-accumulator-if-you've-enabled-that, which is
something that does seem likely to have been unmentioned because this is
the condmod is the only piece that would be using it.

> I do, however, see some rambling text in the BSpec (sadly not the PRM)
> about accumulators, 33-bit precision, and unsigned negates being
> unpredictable.
>
> I guess, in lieu of anything else, these patches are probably okay.
> I'm still not pleased with the situation, but that's not your fault.
> 
> By the way, I mentioned that accumulators had been reworked on
> Ivybridge, so I was nervous about these patches.  That text has been
> removed from the BSpec (grr!) and your patches work fine on IVB.  So, no
> concerns on that front.

OK.  I realized that I wanted to check and be sure that this issue has
existed across generations, so I'll wait to push until then.
-------------- 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/20111006/e57a0ba8/attachment.pgp>