Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1543826 > unrolled thread
| Started by | Joshua Clayton <stillcompiling@gmail.com> |
|---|---|
| First post | 2016-12-17 00:20 +0100 |
| Last post | 2016-12-20 18:30 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v6 2/5] lib: implement __arch_bitrev8x4() Joshua Clayton <stillcompiling@gmail.com> - 2016-12-17 00:20 +0100
Re: [PATCH v6 2/5] lib: implement __arch_bitrev8x4() Will Deacon <will.deacon@arm.com> - 2016-12-19 11:10 +0100
Re: [PATCH v6 2/5] lib: implement __arch_bitrev8x4() Joshua Clayton <stillcompiling@gmail.com> - 2016-12-20 18:30 +0100
| From | Joshua Clayton <stillcompiling@gmail.com> |
|---|---|
| Date | 2016-12-17 00:20 +0100 |
| Subject | [PATCH v6 2/5] lib: implement __arch_bitrev8x4() |
| Message-ID | <sP9Xb-1Cb-5@gated-at.bofh.it> |
Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms
with CONFIG_HAVE_ARCH_BITREVERSE.
ARM platforms just need a byteswap added to the existing __arch_bitrev32()
Amusingly, the mips implementation is exactly the opposite, requiring
removal of the byteswap from its __arch_bitrev32()
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/include/asm/bitrev.h | 6 ++++++
arch/arm64/include/asm/bitrev.h | 6 ++++++
arch/mips/include/asm/bitrev.h | 6 ++++++
include/linux/bitrev.h | 1 +
4 files changed, 19 insertions(+)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
index ec291c3..9482f78 100644
--- a/arch/arm/include/asm/bitrev.h
+++ b/arch/arm/include/asm/bitrev.h
@@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return __arch_bitrev32((u32)x) >> 24;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+ return x;
+}
+
#endif
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
index a5a0c36..1801078 100644
--- a/arch/arm64/include/asm/bitrev.h
+++ b/arch/arm64/include/asm/bitrev.h
@@ -16,4 +16,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return __arch_bitrev32((u32)x) >> 24;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+ return x;
+}
+
#endif
diff --git a/arch/mips/include/asm/bitrev.h b/arch/mips/include/asm/bitrev.h
index bc739a4..9ac6439 100644
--- a/arch/mips/include/asm/bitrev.h
+++ b/arch/mips/include/asm/bitrev.h
@@ -27,4 +27,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return ret;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ u32 ret;
+ asm("bitswap %0, %1" : "=r"(ret) : "r"(x));
+ return ret;
+}
#endif /* __MIPS_ASM_BITREV_H__ */
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 868dcb6..b1cfa1a 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -9,6 +9,7 @@
#define __bitrev32 __arch_bitrev32
#define __bitrev16 __arch_bitrev16
#define __bitrev8 __arch_bitrev8
+#define __bitrev8x4 __arch_bitrev8x4
#else
extern u8 const byte_rev_table[256];
--
2.9.3
[toc] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2016-12-19 11:10 +0100 |
| Message-ID | <sQ33k-6mo-11@gated-at.bofh.it> |
| In reply to | #1543826 |
On Fri, Dec 16, 2016 at 03:17:51PM -0800, Joshua Clayton wrote:
> Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms
> with CONFIG_HAVE_ARCH_BITREVERSE.
> ARM platforms just need a byteswap added to the existing __arch_bitrev32()
> Amusingly, the mips implementation is exactly the opposite, requiring
> removal of the byteswap from its __arch_bitrev32()
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
> arch/arm/include/asm/bitrev.h | 6 ++++++
> arch/arm64/include/asm/bitrev.h | 6 ++++++
> arch/mips/include/asm/bitrev.h | 6 ++++++
> include/linux/bitrev.h | 1 +
> 4 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
> index ec291c3..9482f78 100644
> --- a/arch/arm/include/asm/bitrev.h
> +++ b/arch/arm/include/asm/bitrev.h
> @@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
> return __arch_bitrev32((u32)x) >> 24;
> }
>
> +static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
> +{
> + __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
> + return x;
> +}
> +
> #endif
> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
> index a5a0c36..1801078 100644
> --- a/arch/arm64/include/asm/bitrev.h
> +++ b/arch/arm64/include/asm/bitrev.h
> @@ -16,4 +16,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
> return __arch_bitrev32((u32)x) >> 24;
> }
>
> +static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
> +{
> + __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
This is broken -- you're operating on 64-bit registers. I only noticed
because if you write:
swab32(bitrev32(x))
then GCC generates:
rbit w0, w0
rev w0, w0
so perhaps we should just implement the asm-generic version like that
and not bother with the arch-specific stuff?
Will
[toc] | [prev] | [next] | [standalone]
| From | Joshua Clayton <stillcompiling@gmail.com> |
|---|---|
| Date | 2016-12-20 18:30 +0100 |
| Message-ID | <sQwoF-8wo-5@gated-at.bofh.it> |
| In reply to | #1544417 |
Will,
On 12/19/2016 02:06 AM, Will Deacon wrote:
> On Fri, Dec 16, 2016 at 03:17:51PM -0800, Joshua Clayton wrote:
>> Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms
>> with CONFIG_HAVE_ARCH_BITREVERSE.
>> ARM platforms just need a byteswap added to the existing __arch_bitrev32()
>> Amusingly, the mips implementation is exactly the opposite, requiring
>> removal of the byteswap from its __arch_bitrev32()
>>
>> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
>> ---
>> arch/arm/include/asm/bitrev.h | 6 ++++++
>> arch/arm64/include/asm/bitrev.h | 6 ++++++
>> arch/mips/include/asm/bitrev.h | 6 ++++++
>> include/linux/bitrev.h | 1 +
>> 4 files changed, 19 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
>> index ec291c3..9482f78 100644
>> --- a/arch/arm/include/asm/bitrev.h
>> +++ b/arch/arm/include/asm/bitrev.h
>> @@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
>> return __arch_bitrev32((u32)x) >> 24;
>> }
>>
>> +static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
>> +{
>> + __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
>> + return x;
>> +}
>> +
>> #endif
>> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
>> index a5a0c36..1801078 100644
>> --- a/arch/arm64/include/asm/bitrev.h
>> +++ b/arch/arm64/include/asm/bitrev.h
>> @@ -16,4 +16,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
>> return __arch_bitrev32((u32)x) >> 24;
>> }
>>
>> +static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
>> +{
>> + __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
> This is broken -- you're operating on 64-bit registers. I only noticed
> because if you write:
Ugh. mea culpa. I squinted at the AARCH64 asm and erroneously
believed it to be the same as arm.
> swab32(bitrev32(x))
>
> then GCC generates:
>
> rbit w0, w0
> rev w0, w0
>
> so perhaps we should just implement the asm-generic version like that
> and not bother with the arch-specific stuff?
>
> Will
You are so right.
That is exactly what I will do.
Thanks,
Joshua
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web