[Mesa-dev] [PATCH 1/2] mesa: remove buggy assertions in unpack Z24

[Marek Olšák]

On Mon, Dec 12, 2011 at 2:10 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
>
>
> ----- Original Message -----
>> On Mon, Dec 12, 2011 at 1:24 PM, Jose Fonseca <jfonseca at vmware.com>
>> wrote:
>> > I saw this email yesterday, but did not have time to reply.
>> >
>> > ----- Original Message -----
>> >> On Mon, Nov 21, 2011 at 6:52 PM, Eric Anholt <eric at anholt.net>
>> >> wrote:
>> >> > On Sun, 20 Nov 2011 15:08:55 +0100, Marek Olšák
>> >> > <maraeo at gmail.com>
>> >> > wrote:
>> >> >> unpack_float_z_Z24_X8 fails with -O2 in:
>> >> >> - fbo-blit-d24s8
>> >> >> - fbo-depth-sample-compare
>> >> >> - fbo-readpixels-depth-formats
>> >> >> - glean/depthStencil
>> >> >>
>> >> >> With -O0, it works fine.
>> >> >>
>> >> >> I am removing all the assertions. There's not much point in
>> >> >> having
>> >> >> them,
>> >> >> is there?
>> >> >
>> >> > Is -ffast-math involved at all?
>> >>
>> >> Yes, -fno-fast-math makes the problem go away.
>> >>
>> >> >
>> >> > At a guess, replacing "* scale" with "/ (float)0xffffff" makes
>> >> > the
>> >> > failure happen either way?
>> >>
>> >> Yes. Only using GLdouble fixes it.
>> >>
>> >> It makes sense. The mantissa without the sign has 23 bits, but 24
>> >> bits
>> >> are required to exactly represent 0xffffff if I am not mistaken.
>> >
>> > I'm afraid this is wrong: single precision floating point can
>> > describe 24bits uints (and therefore unorms) without any loss,
>> > because although the mantissa has 23bits, the mantissa is only
>> > used to represent all but the most significant bit, ie., there's
>> > an implicit 24th bit always set to one.
>> >
>> > The fact that -fno-fast-math makes the problem go away is yet
>> > another clear proof that the issue is _not_ lack of precision of
>> > single-precision floating point -- as -fno-fast-math will still
>> > use single-precision floating point numbers.  Actually,
>> > fno-fast-math, it will ensure that all intermediate steps are done
>> > in single precision instead of higher intel x87 80bit floating
>> > points, on the 32bit x86 architecture.  And I suspect the problem
>> > here is that the rounding w/ x80 yields wrong results.
>> >
>> >
>> > I also disagree that using double precision is a good solution.
>> > Either fast-math serves our purposes, or it doesn't.  Using double
>> > precision to work-around issues with single-precision fast-math is
>> > the *worst* thing we could do.
>> >
>> >
>> > Does the assertion failure even happen on 64bits? I doubt, as x64
>> > ABI establishes the use of sse for all floating point, and x87
>> > 80bit floating point will never be used. So effectively this is
>> > making all archs slower.
>> >
>> >
>> > Honestly, I think the patch should be recalled.  And we should
>> > consider disabling fast-math.  And maybe enabling -mfpmath=sse on
>> > 32bits x86 (i.e., require sse).
>>
>> You're probably right. Feel free to revert & fix the issue in some
>> other way.
>
> OK.
>
> Could you please confirm whether the assertions failures you saw were on 32bits or 64bits mode?

32-bit.

Marek