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

[Jose Fonseca]

----- 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?

Jose