Re: [PATCH 3/9] d3dx9: Add 'asin' preshader opcode.

[Matteo Bruni]

2017-05-08 17:39 GMT+02:00 Paul Gofman :
> +/* is_nan() does not work (by design) if compiled with gcc -ffinite-math-only option which is implied by
> -
> - -ffast_math. */
>
> +static BOOL pres_is_nan(double v)
> +{
> - return (*(ULONG64 *)&v & 0x7ff8000000000000) == 0x7ff8000000000000;
>
> +}

I don't think we care about people compiling Wine with broken CFLAGS.
We should just trust the isnan() library function. FWIW we already do
so in other places in Wine.

[PATCH 3/9] d3dx9: Add 'asin' preshader opcode.

[Paul Gofman]

On 05/11/2017 05:42 PM, Matteo Bruni wrote:
> 2017-05-08 17:39 GMT+02:00 Paul Gofman :
> > +/* is_nan() does not work (by design) if compiled with gcc -ffinite-math-only option which is implied by
> > -
> > - -ffast_math. */
> >
> > +static BOOL pres_is_nan(double v)
> > +{
> > - return (*(ULONG64 *)&v & 0x7ff8000000000000) == 0x7ff8000000000000;
> >
> > +}
>
> I don't think we care about people compiling Wine with broken CFLAGS.
> We should just trust the isnan() library function. FWIW we already do
> so in other places in Wine.

I didn't know -ffast-math is not globally supported and though of it as 
possible optimization (which I am successfully using along with some 
other in my build without any problems so far). I will change that.

[PATCH 3/9] d3dx9: Add 'asin' preshader opcode.

[Matteo Bruni]

2017-05-11 17:01 GMT+02:00 Paul Gofman :
> On 05/11/2017 05:42 PM, Matteo Bruni wrote:
> > 2017-05-08 17:39 GMT+02:00 Paul Gofman :
> > > +/* is_nan() does not work (by design) if compiled with gcc
> > > -ffinite-math-only option which is implied by
> > > -
> > > - -ffast_math. */
> > >
> > > +static BOOL pres_is_nan(double v)
> > > +{
> > > - return (*(ULONG64 *)&v & 0x7ff8000000000000) == 0x7ff8000000000000;
> > >
> > > +}
> >
> > I don't think we care about people compiling Wine with broken CFLAGS.
> > We should just trust the isnan() library function. FWIW we already do
> > so in other places in Wine.
>
> I didn't know -ffast-math is not globally supported and though of it as
> possible optimization (which I am successfully using along with some other
> in my build without any problems so far). I will change that.

I'd argue that respecting -ffast-math by ignoring NaNs would be the
correct behavior either way. Certainly you get to keep the pieces of
anything that breaks.

[PATCH 3/9] d3dx9: Add 'asin' preshader opcode.

[Paul Gofman]

On 05/11/2017 06:17 PM, Matteo Bruni wrote:
> 2017-05-11 17:01 GMT+02:00 Paul Gofman :
> > On 05/11/2017 05:42 PM, Matteo Bruni wrote:
> > > 2017-05-08 17:39 GMT+02:00 Paul Gofman :
> > > > +/* is_nan() does not work (by design) if compiled with gcc
> > > > -ffinite-math-only option which is implied by
> > > > -
> > > > - -ffast_math. */
> > > >
> > > > +static BOOL pres_is_nan(double v)
> > > > +{
> > > > - return (*(ULONG64 *)&v & 0x7ff8000000000000) == 0x7ff8000000000000;
> > > >
> > > > +}
> > >
> > > I don't think we care about people compiling Wine with broken CFLAGS.
> > > We should just trust the isnan() library function. FWIW we already do
> > > so in other places in Wine.
> >
> > I didn't know -ffast-math is not globally supported and though of it as
> > possible optimization (which I am successfully using along with some other
> > in my build without any problems so far). I will change that.
>
> I'd argue that respecting -ffast-math by ignoring NaNs would be the
> correct behavior either way. Certainly you get to keep the pieces of
> anything that breaks.

Yes, sure, I will change the patch removing custom is_nan().