sparc: use the new byteorder headers

PATCH In review Harvey Harrison 2 participants 3 messages Aug 19, 2008 SPARC + UltraSPARC (sparc/sparc64)

lore

Share & tools

lore

Harvey Harrison

author

2008-08-19 00:54 UTC #0 lore raw

PATCH 18/23 sparc: use the new byteorder headers

Signed-off-by Harvey Harrison <[email protected]>

---
 arch/sparc/include/asm/byteorder.h |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

arch/sparc/include/asm/byteorder.h +8 ~~-14~~

index bcd83aa..5a70f13 100644
@@ -4,15 +4,14 @@
 #include <asm/types.h>
 #include <asm/asi.h>
 
-#ifdef __GNUC__
+#define __BIG_ENDIAN
 
 #ifdef CONFIG_SPARC32
 #define __SWAB_64_THRU_32__
 #endif
 
 #ifdef CONFIG_SPARC64
-
-static inline __u16 ___arch__swab16p(const __u16 *addr)
+static inline __u16 __arch_swab16p(const __u16 *addr)
 {
 	__u16 ret;

@@ -21,8 +20,9 @@ static inline __u16 ___arch__swab16p(const __u16 *addr)
 			      : "r" (addr), "i" (ASI_PL));
 	return ret;
 }
+#define __arch_swab16p __arch_swab16p
 
-static inline __u32 ___arch__swab32p(const __u32 *addr)
+static inline __u32 __arch_swab32p(const __u32 *addr)
 {
 	__u32 ret;

@@ -31,8 +31,9 @@ static inline __u32 ___arch__swab32p(const __u32 *addr)
 			      : "r" (addr), "i" (ASI_PL));
 	return ret;
 }
+#define __arch_swab32p __arch_swab32p
 
-static inline __u64 ___arch__swab64p(const __u64 *addr)
+static inline __u64 __arch_swab64p(const __u64 *addr)
 {
 	__u64 ret;

@@ -41,17 +42,10 @@ static inline __u64 ___arch__swab64p(const __u64 *addr)
 			      : "r" (addr), "i" (ASI_PL));
 	return ret;
 }
-
-#define __arch__swab16p(x) ___arch__swab16p(x)
-#define __arch__swab32p(x) ___arch__swab32p(x)
-#define __arch__swab64p(x) ___arch__swab64p(x)
+#define __arch_swab64p __arch_swab64p
 
 #endif /* CONFIG_SPARC64 */
 
-#define __BYTEORDER_HAS_U64__
-
-#endif
-
-#include <linux/byteorder/big_endian.h>
+#include <linux/byteorder.h>
 
 #endif /* _SPARC_BYTEORDER_H */

Signature · 1 line

1.6.0.274.g8aacc

David Miller

maintainer

2008-08-19 08:44 UTC #1 lore raw

From: Harvey Harrison <[email protected]>
Date: Mon, 18 Aug 2008 17:48:17 -0700

Quoted 1 line

Signed-off-by Harvey Harrison <[email protected]>

I'm not so sure about this.

If I understand the ___swab*() inlines in linux/swab.h, it has the following priority of swapping methods:

1) If arch defines __arch_swab*(), this is used.

2) If arch defines __arch_swab*p(), variable is popped onto the stack and we do the pointer based operation.

3) Else normal C version is used.

Case #2 is totally disagree with.

Especially for small swaps such as 16-bit the inline expansion of the portable C code is going to be much better than popping the variable onto and then back off the stack.

Sparc 64-bit only provides the __arch_swab*p() routines so #2 is what will in fact be used here.

So NACK based upon that analysis.

Harvey Harrison

author

2008-08-19 19:02 UTC #2 lore raw

Quoted 7 lines · David Miller

From: Harvey Harrison <[email protected]>
Date: Mon, 18 Aug 2008 17:48:17 -0700
Quoted 1 line
Signed-off-by Harvey Harrison <[email protected]>
I'm not so sure about this.

Quoted 7 lines

If I understand the ___swab*() inlines in linux/swab.h, it has the following priority of swapping methods:
1) If arch defines __arch_swab*(), this is used.
2) If arch defines __arch_swab*p(), variable is popped onto the stack and we do the pointer based operation.

2a) If defined(__SWAB_64_THRU_32__) swab64 uses swab32 internally.

Quoted 3 lines

3) Else normal C version is used.

Your above understanding is correct.

Quoted 6 lines

Case #2 is totally disagree with.
Especially for small swaps such as 16-bit the inline expansion of the portable C code is going to be much better than popping the variable onto and then back off the stack.

I thought gcc wasn't too bad for this case these days for attribute_const inlines? But without evidence to back it up, you're right that the generic C version should just be used if an arch hasn't provided an override.

Quoted 4 lines

Sparc 64-bit only provides the __arch_swab*p() routines so #2 is what will in fact be used here.
So NACK based upon that analysis.

From: Harvey Harrison <[email protected]>
Subject: [PATCH] byteorder: use generic C version for value byteswapping

David Miller noted that popping the variable on and back off the stack will probably be more expensive than just using the generic C byteswapping code. Remove the fallback to the swap-from-pointer helper when no arch override for the value byteswap has been defined.

Signed-off-by Harvey Harrison <[email protected]>

---
 include/linux/swab.h |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

include/linux/swab.h +0 ~~-10~~

index 270d5c2..bbed279 100644
@@ -47,8 +47,6 @@ static inline __attribute_const__ __u16 ___swab16(__u16 val)
 {
 #ifdef __arch_swab16
 	return __arch_swab16(val);
-#elif defined(__arch_swab16p)
-	return __arch_swab16p(&val);
 #else
 	return __const_swab16(val);
 #endif

@@ -58,8 +56,6 @@ static inline __attribute_const__ __u32 ___swab32(__u32 val)
 {
 #ifdef __arch_swab32
 	return __arch_swab32(val);
-#elif defined(__arch_swab32p)
-	return __arch_swab32p(&val);
 #else
 	return __const_swab32(val);
 #endif

@@ -69,8 +65,6 @@ static inline __attribute_const__ __u64 ___swab64(__u64 val)
 {
 #ifdef __arch_swab64
 	return __arch_swab64(val);
-#elif defined(__arch_swab64p)
-	return __arch_swab64p(&val);
 #elif defined(__SWAB_64_THRU_32__)
 	__u32 h = val >> 32;
 	__u32 l = val & ((1ULL << 32) - 1);

@@ -84,8 +78,6 @@ static inline __attribute_const__ __u32 ___swahw32(__u32 val)
 {
 #ifdef __arch_swahw32
 	return __arch_swahw32(val);
-#elif defined(__arch_swahw32p)
-	return __arch_swahw32p(&val);
 #else
 	return __const_swahw32(val);
 #endif

@@ -95,8 +87,6 @@ static inline __attribute_const__ __u32 ___swahb32(__u32 val)
 {
 #ifdef __arch_swahb32
 	return __arch_swahb32(val);
-#elif defined(__arch_swahb32p)
-	return __arch_swahb32p(&val);
 #else
 	return __const_swahb32(val);
 #endif

Signature · 1 line

1.6.0.274.g8aacc