2013-01-13 19:41:32

by Cristian Rodríguez

[permalink] [raw]
Subject: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

In x86, it will not make much difference but other targets that
are not covered by the old code will be able to generate better code.

Signed-off-by: Cristian Rodríguez <[email protected]>
---
lib/ext2fs/bitops.h | 91 +++++++++++------------------------------------------
1 file changed, 18 insertions(+), 73 deletions(-)

diff --git a/lib/ext2fs/bitops.h b/lib/ext2fs/bitops.h
index 1559539..8437b1e 100644
--- a/lib/ext2fs/bitops.h
+++ b/lib/ext2fs/bitops.h
@@ -10,33 +10,24 @@
* %End-Header%
*/

-#ifdef WORDS_BIGENDIAN
-#define ext2fs_cpu_to_le64(x) ext2fs_swab64((x))
-#define ext2fs_le64_to_cpu(x) ext2fs_swab64((x))
-#define ext2fs_cpu_to_le32(x) ext2fs_swab32((x))
-#define ext2fs_le32_to_cpu(x) ext2fs_swab32((x))
-#define ext2fs_cpu_to_le16(x) ext2fs_swab16((x))
-#define ext2fs_le16_to_cpu(x) ext2fs_swab16((x))
-#define ext2fs_cpu_to_be64(x) ((__u64)(x))
-#define ext2fs_be64_to_cpu(x) ((__u64)(x))
-#define ext2fs_cpu_to_be32(x) ((__u32)(x))
-#define ext2fs_be32_to_cpu(x) ((__u32)(x))
-#define ext2fs_cpu_to_be16(x) ((__u16)(x))
-#define ext2fs_be16_to_cpu(x) ((__u16)(x))
-#else
-#define ext2fs_cpu_to_le64(x) ((__u64)(x))
-#define ext2fs_le64_to_cpu(x) ((__u64)(x))
-#define ext2fs_cpu_to_le32(x) ((__u32)(x))
-#define ext2fs_le32_to_cpu(x) ((__u32)(x))
-#define ext2fs_cpu_to_le16(x) ((__u16)(x))
-#define ext2fs_le16_to_cpu(x) ((__u16)(x))
-#define ext2fs_cpu_to_be64(x) ext2fs_swab64((x))
-#define ext2fs_be64_to_cpu(x) ext2fs_swab64((x))
-#define ext2fs_cpu_to_be32(x) ext2fs_swab32((x))
-#define ext2fs_be32_to_cpu(x) ext2fs_swab32((x))
-#define ext2fs_cpu_to_be16(x) ext2fs_swab16((x))
-#define ext2fs_be16_to_cpu(x) ext2fs_swab16((x))
-#endif
+#include <byteswap.h>
+#include <endian.h>
+
+#define ext2fs_swab32(x) bswap_32(x)
+#define ext2fs_swab16(x) bswap_16(x)
+#define ext2fs_swab64(x) bswap_64(x)
+#define ext2fs_cpu_to_le64(x) htole64((x))
+#define ext2fs_le64_to_cpu(x) le64toh((x))
+#define ext2fs_cpu_to_le32(x) htole32((x))
+#define ext2fs_le32_to_cpu(x) le32toh((x))
+#define ext2fs_cpu_to_le16(x) htole16((x))
+#define ext2fs_le16_to_cpu(x) le16toh((x))
+#define ext2fs_cpu_to_be64(x) htobe64((x))
+#define ext2fs_be64_to_cpu(x) be64toh((x))
+#define ext2fs_cpu_to_be32(x) htobe32((x))
+#define ext2fs_be32_to_cpu(x) be32toh((x))
+#define ext2fs_cpu_to_be16(x) htobe16((x))
+#define ext2fs_be16_to_cpu(x) be16toh((x))

/*
* EXT2FS bitmap manipulation routines.
@@ -319,54 +310,11 @@ _INLINE_ int ext2fs_test_bit(unsigned int nr, const void * addr)
return oldbit;
}

-_INLINE_ __u32 ext2fs_swab32(__u32 val)
-{
-#ifdef EXT2FS_REQUIRE_486
- __asm__("bswap %0" : "=r" (val) : "0" (val));
-#else
- __asm__("xchgb %b0,%h0\n\t" /* swap lower bytes */
- "rorl $16,%0\n\t" /* swap words */
- "xchgb %b0,%h0" /* swap higher bytes */
- :"=q" (val)
- : "0" (val));
-#endif
- return val;
-}
-
-_INLINE_ __u16 ext2fs_swab16(__u16 val)
-{
- __asm__("xchgb %b0,%h0" /* swap bytes */ \
- : "=q" (val) \
- : "0" (val)); \
- return val;
-}
-
#undef EXT2FS_ADDR

#endif /* i386 */


-#if !defined(_EXT2_HAVE_ASM_SWAB_)
-
-_INLINE_ __u16 ext2fs_swab16(__u16 val)
-{
- return (val >> 8) | (val << 8);
-}
-
-_INLINE_ __u32 ext2fs_swab32(__u32 val)
-{
- return ((val>>24) | ((val>>8)&0xFF00) |
- ((val<<8)&0xFF0000) | (val<<24));
-}
-
-#endif /* !_EXT2_HAVE_ASM_SWAB */
-
-_INLINE_ __u64 ext2fs_swab64(__u64 val)
-{
- return (ext2fs_swab32(val >> 32) |
- (((__u64)ext2fs_swab32(val & 0xFFFFFFFFUL)) << 32));
-}


2013-01-14 14:13:58

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

On Sat, Jan 12, 2013 at 04:32:25PM -0300, Cristian Rodr?guez wrote:
> In x86, it will not make much difference but other targets that
> are not covered by the old code will be able to generate better code.
>
> Signed-off-by: Cristian Rodr?guez <[email protected]>

The problem is that bswap_{16,32,64}() and the existence of
<byteswap.h> is not guaranted by any standard that I'm not aware of.
A quick Google search indicates that it's not available for the
following platforms:

* Mac OS X 10.5
* FreeBSD 6.0
* NetBSD 5.0
* OpenBSD 3.8
* Minix 3.1.8
* AIX 5.1
* HP-UX 11
* IRIX 6.5
* OSF/1 5.1
* Solaris 11 2011-11
* Cygwin
* mingw
* MSVC 9
* Interix 3.5
* BeOS

So the only way we could use bswap_{16,32,64} if there is a proper
configure.in test for them.

Regards,

- Ted

2013-01-14 14:20:07

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

On Mon, Jan 14, 2013 at 09:13:56AM -0500, Theodore Ts'o wrote:
> The problem is that bswap_{16,32,64}() and the existence of
> <byteswap.h> is not guaranted by any standard that I'm not aware of.
> A quick Google search indicates that it's not available for the
> following platforms....

P.S. This "all the world's a Linux platforms" attitude is why a lot
of people are very snide about how many Linux userspace programmers
will introduced vast bloated, slow, infuriating autoconf and automake
infrastructure, and then use Linux'isms that completely break all
portability for anything other than Linux systems.... at which point,
why bother using autoconf and automake?

- Ted

2013-01-14 15:34:38

by Cristian Rodríguez

[permalink] [raw]
Subject: Re: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

On 01/14/2013 11:20 AM, Theodore Ts'o wrote:

> why bother using autoconf and automake

For a very simple reason, fixing hand written makefiles is the number 1
burden in distribution mainteniance.

Any autotools stuff provided is always better and causes less pain than
ad-hoc build scripts.

This is why you see frecuent requests to use autotools, because hand
crafted stuff breaks literally everyday, just ask any distribution
packager if you dont believe me.




2013-01-14 21:56:26

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

On Mon, Jan 14, 2013 at 12:33:31PM -0300, Cristian Rodr?guez wrote:
> On 01/14/2013 11:20 AM, Theodore Ts'o wrote:
>
> >why bother using autoconf and automake
>
> For a very simple reason, fixing hand written makefiles is the
> number 1 burden in distribution mainteniance.
>
> Any autotools stuff provided is always better and causes less pain
> than ad-hoc build scripts.

Maybe for distribution people (although I haven't had that many
complaints with my hand-written Makefile.in files). But automake
causes ***huge*** amounts of pain for developers, especially when they
want to run binaries out of the build tree for debugging purposes or
for regression testing.

I hate automake almost as much as I dispise GNOME 3.

- Ted

2013-01-15 01:08:26

by Cristian Rodríguez

[permalink] [raw]
Subject: Re: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

On 01/14/2013 11:13 AM, Theodore Ts'o wrote:
> On Sat, Jan 12, 2013 at 04:32:25PM -0300, Cristian Rodr?guez wrote:
>> In x86, it will not make much difference but other targets that
>> are not covered by the old code will be able to generate better code.
>>
>> Signed-off-by: Cristian Rodr?guez <[email protected]>
>
> The problem is that bswap_{16,32,64}() and the existence of
> <byteswap.h> is not guaranted by any standard that I'm not aware of.
> A quick Google search indicates that it's not available for the
> following platforms:
>
> * Mac OS X 10.5
> * FreeBSD 6.0
> * NetBSD 5.0
> * OpenBSD 3.8
> * Minix 3.1.8
> * AIX 5.1
> * HP-UX 11
> * IRIX 6.5
> * OSF/1 5.1
> * Solaris 11 2011-11
> * Cygwin
> * mingw
> * MSVC 9
> * Interix 3.5
> * BeOS
>
> So the only way we could use bswap_{16,32,64} if there is a proper
> configure.in test for them.

The code does not compile anyway at least in mingw (I just tried) and
probably in several other targets anyway.. with or without my patch :-)

I wonder *where* the developers of this other systems are and if they
are willing to submit any improvement or fixes for the software.


2013-01-15 02:05:22

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

On Mon, Jan 14, 2013 at 10:07:18PM -0300, Cristian Rodr?guez wrote:
>
> The code does not compile anyway at least in mingw (I just tried)
> and probably in several other targets anyway.. with or without my
> patch :-)

I know for certain that the code compiles on MacOS, *BSD, and Solaris...

- Ted