diff mbox

lib/ext2fs/bitops.h: Use the optmized/documented byteswapping routines

Message ID 1358019145-2774-1-git-send-email-crrodriguez@opensuse.org
State Rejected, archived
Headers show

Commit Message

Cristian Rodríguez Jan. 12, 2013, 7:32 p.m. UTC 
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 <crrodriguez@opensuse.org>
---
 lib/ext2fs/bitops.h | 91 +++++++++++------------------------------------------
 1 file changed, 18 insertions(+), 73 deletions(-)

Comments

Theodore Ts'o Jan. 14, 2013, 2:13 p.m. UTC | #1 
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 <crrodriguez@opensuse.org>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o Jan. 14, 2013, 2:20 p.m. UTC | #2 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cristian Rodríguez Jan. 14, 2013, 3:33 p.m. UTC | #3 
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.



--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o Jan. 14, 2013, 9:56 p.m. UTC | #4 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cristian Rodríguez Jan. 15, 2013, 1:07 a.m. UTC | #5 
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 <crrodriguez@opensuse.org>
>
> 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.



--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o Jan. 15, 2013, 2:05 a.m. UTC | #6 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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));
-}
-
 _INLINE_ int ext2fs_mark_block_bitmap(ext2fs_block_bitmap bitmap,
 				       blk_t block)
 {
@@ -656,8 +604,5 @@  extern void ext2fs_fast_set_bit(unsigned int nr,void * addr);
 extern void ext2fs_fast_clear_bit(unsigned int nr, void * addr);
 extern void ext2fs_fast_set_bit64(__u64 nr,void * addr);
 extern void ext2fs_fast_clear_bit64(__u64 nr, void * addr);
-extern __u16 ext2fs_swab16(__u16 val);
-extern __u32 ext2fs_swab32(__u32 val);
-extern __u64 ext2fs_swab64(__u64 val);
 #endif