[Mesa-dev] [PATCH 2/3] i965/fs: Fix comparisions with uint negation.
eric at anholt.net
Thu Oct 6 13:47:51 PDT 2011
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
> 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
Size: 197 bytes
Desc: not available
More information about the mesa-dev