[PATCH v3] dix: add utility functions for double to/fro FP1616/FP3232 conversion

[Michel Dänzer]

On Mit, 2011-10-05 at 15:02 -0700, Jeremy Huddleston wrote: 
> Co-authored-by: Jeremy Huddleston <jeremyhu at apple.com>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

[...]

> +    frac = ldexp((double)(in & 0xffff), -16);

The compiler can't optimize away these ldexp() calls.

> +    tmp = (in - integral) * (1ULL << 16); /* optimized from ldexp(in - integral, 16) */

Why use 1ULL for a 16-bit shift that ends up in a 32-bit int?

> +    for (i = 0x7FFFF; i < 0x7FFFFFFF; i <<= 1) {

This is an infinite loop on 32-bit. (Didn't notice it either until I ran
the patched test)


FWIW, below is how I'd write these helpers. Other than the issues
pointed out so far, these also avoid floating point division and
conversion from integer integral back to floating point for the
calculation of frac.

BTW, maybe these should be inline functions?

These pass the patched test/input as well as
test/xi2/protocol-{eventconvert,xiquerydevice} with Xi/xiquerdydevice.c,
dix/eventconvert.c and test/xi2/protocol-eventconvert.c patched to use
double_to_fp3232().


double
fp1616_to_double(FP1616 in)
{
    return (double)(in >> 16) + (double)(in & 0xffff) * (1.0 / (1 << 16));
}

double
fp3232_to_double(FP3232 in)
{
    return (double)in.integral + (double)in.frac * (1.0 / (1ULL << 32));
}


FP1616
double_to_fp1616(double in)
{
    double flr = floor(in);

    return (int32_t)flr << 16 | (uint16_t)((in - flr) * (1 << 16));
}

FP3232
double_to_fp3232(double in)
{
    double flr = floor(in);
    FP3232 ret;

    ret.integral = flr;
    ret.frac = (in - flr) * (1ULL << 32);

    return ret;
}


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer