Skip to contentSkip to conversation

powerpc: merge byteorder.h

PATCH In review Becky Bruce 2 participants 3 messages Sep 27, 2005 – Oct 5, 2005

lore
Share & tools
lore

Becky Bruce

author
#0 lore raw

PATCH 1/1 powerpc: merge byteorder.h

powerpc: Merge byteorder.h

Essentially adopts the 64-bit version of this file. The 32-bit version had been using unsigned ints for arguments/return values that were actually only 16 bits - the new file uses __u16 for these items as in the 64-bit version of the header. The order of some of the asm constraints in the 64-bit version was slightly different than the 32-bit version, but they produce identical code.

Signed-off-by Becky Bruce <[email protected]>
Signed-off-by Kumar Gala <[email protected]>

---
commit 01344596fdecbe2b97e122b6a50570a19218cd2f
tree ccdf20534198e69459d95f21b8f45aafc00e8238
parent d407c9f3f6f3c84d9daec257f9a2550aacbd2892
author Becky Bruce <[email protected]> Tue, 27 Sep 2005 14:04:44 -0500 committer Becky Bruce <[email protected]> Tue, 27 Sep 2005 14:04:44 -0500

 include/asm-powerpc/byteorder.h |   89 +++++++++++++++++++++++++++++++++++++++
 include/asm-ppc/byteorder.h     |   76 ---------------------------------
 include/asm-ppc64/byteorder.h   |   86 --------------------------------------
 3 files changed, 89 insertions(+), 162 deletions(-)
include/asm-powerpc/byteorder.h new file +89 -0
new file mode 100644
@@ -0,0 +1,89 @@
+#ifndef _ASM_POWERPC_BYTEORDER_H
+#define _ASM_POWERPC_BYTEORDER_H
+
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <asm/types.h>
+#include <linux/compiler.h>
+
+#ifdef __GNUC__
+#ifdef __KERNEL__
+
+static __inline__ __u16 ld_le16(const volatile __u16 *addr)
+{
+	__u16 val;
+
+	__asm__ __volatile__ ("lhbrx %0,0,%1" : "=r" (val) : "r" (addr), "m" (*addr));
+	return val;
+}
+
+static __inline__ void st_le16(volatile __u16 *addr, const __u16 val)
+{
+	__asm__ __volatile__ ("sthbrx %1,0,%2" : "=m" (*addr) : "r" (val), "r" (addr));
+}
+
+static __inline__ __u32 ld_le32(const volatile __u32 *addr)
+{
+	__u32 val;
+
+	__asm__ __volatile__ ("lwbrx %0,0,%1" : "=r" (val) : "r" (addr), "m" (*addr));
+	return val;
+}
+
+static __inline__ void st_le32(volatile __u32 *addr, const __u32 val)
+{
+	__asm__ __volatile__ ("stwbrx %1,0,%2" : "=m" (*addr) : "r" (val), "r" (addr));
+}
+
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 value)
+{
+	__u16 result;
+
+	__asm__("rlwimi %0,%1,8,16,23"
+	    : "=r" (result)
+	    : "r" (value), "0" (value >> 8));
+	return result;
+}
+
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 value)
+{
+	__u32 result;
+
+	__asm__("rlwimi %0,%1,24,16,23\n\t"
+	    "rlwimi %0,%1,8,8,15\n\t"
+	    "rlwimi %0,%1,24,0,7"
+	    : "=r" (result)
+	    : "r" (value), "0" (value >> 24));
+	return result;
+}
+
+#define __arch__swab16(x) ___arch__swab16(x)
+#define __arch__swab32(x) ___arch__swab32(x)
+
+/* The same, but returns converted value from the location pointer by addr. */
+#define __arch__swab16p(addr) ld_le16(addr)
+#define __arch__swab32p(addr) ld_le32(addr)
+
+/* The same, but do the conversion in situ, ie. put the value back to addr. */
+#define __arch__swab16s(addr) st_le16(addr,*addr)
+#define __arch__swab32s(addr) st_le32(addr,*addr)
+
+#endif /* __KERNEL__ */
+
+#ifndef __STRICT_ANSI__
+#define __BYTEORDER_HAS_U64__
+#ifndef __powerpc64__
+#define __SWAB_64_THRU_32__
+#endif /* __powerpc64__ */
+#endif /* __STRICT_ANSI__ */
+
+#endif /* __GNUC__ */
+
+#include <linux/byteorder/big_endian.h>
+
+#endif /* _ASM_POWERPC_BYTEORDER_H */
include/asm-ppc/byteorder.h deleted +0 -76

76 lines not on this page · raw message

include/asm-ppc64/byteorder.h deleted +0 -86

86 lines not on this page · raw message

#1 lore raw
Quoted 70 lines · Becky Bruce

powerpc: Merge byteorder.h

Essentially adopts the 64-bit version of this file. The 32-bit version had been using unsigned ints for arguments/return values that were actually only 16 bits - the new file uses __u16 for these items as in the 64-bit version of the header. The order of some of the asm constraints in the 64-bit version was slightly different than the 32-bit version, but they produce identical code.

Signed-off-by Becky Bruce <[email protected]>
Signed-off-by Kumar Gala <[email protected]>

---
commit 01344596fdecbe2b97e122b6a50570a19218cd2f
tree ccdf20534198e69459d95f21b8f45aafc00e8238
parent d407c9f3f6f3c84d9daec257f9a2550aacbd2892
author Becky Bruce <[email protected]> Tue, 27 Sep 2005 14:04:44 -0500 committer Becky Bruce <[email protected]> Tue, 27 Sep 2005 14:04:44 -0500

 include/asm-powerpc/byteorder.h |   89 +++++++++++++++++++++++++++++++++++++++
 include/asm-ppc/byteorder.h     |   76 ---------------------------------
 include/asm-ppc64/byteorder.h   |   86 --------------------------------------
 3 files changed, 89 insertions(+), 162 deletions(-)
include/asm-powerpc/byteorder.h new file +41 -0
new file mode 100644
@@ -0,0 +1,89 @@
+#ifndef _ASM_POWERPC_BYTEORDER_H
+#define _ASM_POWERPC_BYTEORDER_H
+
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <asm/types.h>
+#include <linux/compiler.h>
+
+#ifdef __GNUC__
+#ifdef __KERNEL__
+
+static __inline__ __u16 ld_le16(const volatile __u16 *addr)
+{
+	__u16 val;
+
+	__asm__ __volatile__ ("lhbrx %0,0,%1" : "=r" (val) : "r" (addr), "m" (*addr));
+	return val;
+}
+
+static __inline__ void st_le16(volatile __u16 *addr, const __u16 val)
+{
+	__asm__ __volatile__ ("sthbrx %1,0,%2" : "=m" (*addr) : "r" (val), "r" (addr));
+}
+
+static __inline__ __u32 ld_le32(const volatile __u32 *addr)
+{
+	__u32 val;
+
+	__asm__ __volatile__ ("lwbrx %0,0,%1" : "=r" (val) : "r" (addr), "m" (*addr));
+	return val;
+}
+
+static __inline__ void st_le32(volatile __u32 *addr, const __u32 val)
+{
+	__asm__ __volatile__ ("stwbrx %1,0,%2" : "=m" (*addr) : "r" (val), "r" (addr));
+}

include/asm-powerpc/byteorder.h line 51

Quoted 10 lines
include/asm-powerpc/byteorder.h new file +10 -0
+
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 value)
+{
+	__u16 result;
+
+	__asm__("rlwimi %0,%1,8,16,23"
+	    : "=r" (result)
+	    : "r" (value), "0" (value >> 8));
+	return result;
+}

This needs 2 registers where one is enough and often adds a truncation to 16 bit of results in the generated code. Consider instead:

	__u32 result;
	__asm__("	rlwimi %0,%0,16,8,15 # ??12->?212\n"
		: =r(result) : "0"(value));
	return (__u16)(result>>8);

GCC will combine the result>>8 with the truncation to the lower sixteen bits, often saving one instruction.

This will likely generate worse code if you really need to keep both the original value and the swapped one, but this should not be the common case.

include/asm-powerpc/byteorder.h line 63

Quoted 12 lines
include/asm-powerpc/byteorder.h new file +12 -0
+
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 value)
+{
+	__u32 result;
+
+	__asm__("rlwimi %0,%1,24,16,23\n\t"
+	    "rlwimi %0,%1,8,8,15\n\t"
+	    "rlwimi %0,%1,24,0,7"
+	    : "=r" (result)
+	    : "r" (value), "0" (value >> 24));
+	return result;
+}

That one can be improved too, in only needs one rotlwi (rlwinm withou any mask) and 2 rlwimi:

__asm__(" rotlwi %0,%1,8 # 1234 -> 2341\n"

	"	rlwimi %0,%1,24,0,7          \n"
	"	rlwimi %0,%1,24,16,23        \n"
	: "=&r" (result) : r(value))

Notes:

- reformat as you like, but I prefer the tab at the beginning of every line, otherwise the assembly output does not look right. I always add a \n at the end just in case (do all versions of GCC add it or not?).

- the earlyclobber (&) is really necessary otherwise GCC might allocate the same register for input and output for __arch_swab32.

What should really be implemented in GCC is a recognizer for the manipulations that can easily be mapped to a single rlwimi instruction. But it is too complex for the standard combiner IIRC.

Regards,
Gabriel

#2 lore raw

PATCH 1/1 powerpc: improved byte swapping functions

From: Gabriel Paubert <[email protected]>

The previous versions of ___arch__swab16 and ___arch__swab32 were not optimal. In most cases the code can be made shorter and faster with this patch.

Signed-off-by Gabriel Paubert <[email protected]>

---

Additional notes:

1) for ___arch__swab16, the trick is to let the compiler generate a single rlwinm instruction for the final right shift and cast.

2) For ___arch_swab32, the rotated value passed as a parameter already has 2 bytes at the right place, so only 2 rlwimi instructions are necessary to complete the byte swap.

3) edit if you don't like the formatting of the result.

4) I've been reading the thread about how to format patches and I hope that I got it right. But I believe that the diffstat output is overkill for such a small patch.

Regards,
Gabriel

include/asm-powerpc/byteorder.h +8 -9
@@ -42,23 +42,22 @@ static __inline__ void st_le32(volatile 
 
 static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 value)
 {
-	__u16 result;
+	__u32 tmp;
 
-	__asm__("rlwimi %0,%1,8,16,23"
-	    : "=r" (result)
-	    : "r" (value), "0" (value >> 8));
-	return result;
+	__asm__("rlwimi %0,%0,16,8,15"
+		: "=r" (tmp) : "0" (value));
+	return (__u16)(tmp>>8);
 }
 
 static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 value)
 {
 	__u32 result;
 
-	__asm__("rlwimi %0,%1,24,16,23\n\t"
-	    "rlwimi %0,%1,8,8,15\n\t"
-	    "rlwimi %0,%1,24,0,7"
+	__asm__(
+"	rlwimi %0,%1,24,16,23\n"
+"	rlwimi %0,%1,24,0,7\n"
 	    : "=r" (result)
-	    : "r" (value), "0" (value >> 24));
+	    : "r" (value), "0" ((value >> 24)|(value<<8)));
 	return result;
 }