[MPlayer-cvslog] r24067 - in trunk: Changelog Copyright configure dvdread/Makefile dvdread/bswap.h dvdread/dvd_input.c dvdread/dvd_input.h dvdread/dvd_reader.c dvdread/dvd_reader.h dvdread/dvd_udf.c dvdread/dvd_udf.h dvdread/dvdread_internal.h dvdread/ifo_print.c dvdread/ifo_read.c dvdread/ifo_types.h dvdread/nav_print.c dvdread/nav_read.c dvdread/nav_types.h stream/stream_dvd_common.c

[Reimar Doeffinger]

Hello,
On Thu, Aug 16, 2007 at 12:52:17PM +0200, Diego Biurrun wrote:
> On Thu, Aug 16, 2007 at 10:30:33AM +0200, Reimar Doeffinger wrote:
> > On Wed, Aug 15, 2007 at 01:47:22PM +0200, diego wrote:
> > [...]
> > > @@ -61,7 +61,19 @@
> > >  #include <sys/endian.h>
> > >  #define B2N_16(x) x = be16toh(x)
> > >  #define B2N_32(x) x = be32toh(x)
> > > +#if __FreeBSD_version >= 500000
> > >  #define B2N_64(x) x = be64toh(x)
> > > +#else
> > > +#define B2N_64(x)                               \
> > > +  x = ((((x) & 0xff00000000000000) >> 56) |     \
> > > +       (((x) & 0x00ff000000000000) >> 40) |     \
> > > +       (((x) & 0x0000ff0000000000) >> 24) |     \
> > > +       (((x) & 0x000000ff00000000) >>  8) |     \
> > > +       (((x) & 0x00000000ff000000) <<  8) |     \
> > > +       (((x) & 0x0000000000ff0000) << 24) |     \
> > > +       (((x) & 0x000000000000ff00) << 40) |     \
> > > +       (((x) & 0x00000000000000ff) << 56))
> > > +#endif /* _FreeBSD_version >= 500000 */
> > 
> > I just hope we can get rid of this stuff soon, this must be about the
> > worst possible implementation of this in every respect.
> 
> You mean the FreeBSD version check?  Yes, horrible ...

No, I mean the fallback B2N_64. First of all it causes loads of
warnings, because the constants miss ULL specifier.
Secondly, on Power5 this takes 48 instructions, while our equivalent
bswap_64 (also pure C) needs 21 with -O3 (note there may be a constant
offset for those two values, I don't know power5 asm well enough to know
which parts still belong to the stack frame stuff etc.).

Greetings,
Reimar Doeffinger