[PATCH] Adding RBIT gcc builtin for ARM

[PATCH] Adding RBIT gcc builtin for ARM

[Ayan Shafqat]

The attached patch contains __builtin_arm_rbit which generates RBIT 
instruction for ARM targets.

Please let me know if you any questions or comments, or commit this 
patch for me as I do not have write access to SVN.

Thanks
Ayan

commit a692b5b4965840babbdaf5e2b9b1feb1995d351d
Author: Ayan Shafqat <ayan.x.shafqat@gmail.com>
Date:   Mon Jun 17 21:46:54 2019 -0400

     Implementing RBIT builtin as described in ACLE doc

     ARM's RBIT instruction is used to reverse the bit order
     of a word. This is present in ARMv6 and above in both
     ARM and Thumb modes. This is also specified as an intrinsic
     function in ACLE documentation.

     This commit implements the GCC builtin for ARM target for
     RBIT instruction, __builtin_arm_rbit. Also, this implements
     the intrinsic functions as stated in ARM ACLE documentation,
     which are listed below:

     uint32_t __rbit(uint32_t x);
     unsigned long __rbitl(unsigned long x);
     uint64_t __rbitll(uint64_t x);

     Note: __rbitll is implemented as two calls to __rbit. I know
     this is not how it's done in AArch64, but this is what I can
     do for now.

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index ae582172ab9..83dcb7b411c 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -11568,6 +11568,13 @@
    [(set_attr "predicable" "yes")
     (set_attr "type" "clz")])

+(define_insn "rbit"
+  [(set (match_operand:SI 0 "s_register_operand" "=r")
+	(unspec:SI [(match_operand:SI 1 "s_register_operand" "r")] UNSPEC_RBIT))]
+  "TARGET_32BIT && arm_arch_thumb2"
+  "rbit%?\\t%0, %1"
+  [(set_attr "predicable" "yes")])
+
  (define_insn "rbitsi2"
    [(set (match_operand:SI 0 "s_register_operand" "=r")
  	(unspec:SI [(match_operand:SI 1 "s_register_operand" "r")] UNSPEC_RBIT))]
diff --git a/gcc/config/arm/arm_acle.h b/gcc/config/arm/arm_acle.h
index 2c7acc698ea..ce1b102444b 100644
--- a/gcc/config/arm/arm_acle.h
+++ b/gcc/config/arm/arm_acle.h
@@ -168,6 +168,29 @@ __arm_mrrc2 (const unsigned int __coproc, const 
unsigned int __opc1,
  {
    return __builtin_arm_mrrc2 (__coproc, __opc1,  __CRm);
  }
+
+__extension__ static __inline uint32_t __attribute__ ((__always_inline__))
+__rbit(uint32_t __op1)
+{
+  return __builtin_arm_rbit(__op1);
+}
+
+__extension__ static __inline uint64_t __attribute__ ((__always_inline__))
+__rbitll(uint64_t __op1)
+{
+  return (((uint64_t)__rbit(__op1)) << 32U) | __rbit(__op1 >> 32U);
+}
+
+__extension__ static __inline unsigned long __attribute__ 
((__always_inline__))
+__rbitl(unsigned long __op1)
+{
+#if __SIZEOF_LONG__ == 4
+  return __rbit(__op1);
+#else
+  return __rbitll(__op1);
+#endif
+}
+
  #endif /* __ARM_ARCH >= 6.  */
  #endif /* __ARM_ARCH >= 6 ||  defined (__ARM_ARCH_5TE__).  */
  #endif /*  __ARM_ARCH >= 5.  */
diff --git a/gcc/config/arm/arm_acle_builtins.def 
b/gcc/config/arm/arm_acle_builtins.def
index b2438d66da2..ecb3be491fc 100644
--- a/gcc/config/arm/arm_acle_builtins.def
+++ b/gcc/config/arm/arm_acle_builtins.def
@@ -24,6 +24,7 @@ VAR1 (UBINOP, crc32w, si)
  VAR1 (UBINOP, crc32cb, si)
  VAR1 (UBINOP, crc32ch, si)
  VAR1 (UBINOP, crc32cw, si)
+VAR1 (UBINOP, rbit, si)
  VAR1 (CDP, cdp, void)
  VAR1 (CDP, cdp2, void)
  VAR1 (LDC, ldc, void)
diff --git a/gcc/testsuite/gcc.target/arm/acle/rbit.c 
b/gcc/testsuite/gcc.target/arm/acle/rbit.c
new file mode 100644
index 00000000000..7803dd33615
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/acle/rbit.c
@@ -0,0 +1,18 @@
+/* Test the crc32d ACLE intrinsic.  */
+
+/* { dg-do assemble } */
+/* { dg-require-effective-target arm_crc_ok } */
+/* { dg-options "-save-temps -O0" } */
+/* { dg-add-options arm_crc } */
+
+#include "arm_acle.h"
+
+void test_rbit (void)
+{
+  uint32_t out_uint32_t;
+  uint32_t arg0_uint32_t;
+
+  out_uint32_t = __rbit (arg0_uint32_t);
+}
+
+/* { dg-final { scan-assembler-times "rbit\t...?, ...?\n" 2 } } */

Re: [PATCH] Adding RBIT gcc builtin for ARM

[Kyrill Tkachov]

Hi Ayan,

On 6/20/19 4:40 AM, Ayan Shafqat wrote:
> The attached patch contains __builtin_arm_rbit which generates RBIT
> instruction for ARM targets.
>
> Please let me know if you any questions or comments, or commit this
> patch for me as I do not have write access to SVN.
>
Thanks for the patch.

Before we can review this, do you have a copyright assignment in place 
as described at https://gcc.gnu.org/contribute.html ?

Thanks,

Kyrill

> Thanks
> Ayan
>
> commit a692b5b4965840babbdaf5e2b9b1feb1995d351d
> Author: Ayan Shafqat <ayan.x.shafqat@gmail.com>
> Date:   Mon Jun 17 21:46:54 2019 -0400
>
>      Implementing RBIT builtin as described in ACLE doc
>
>      ARM's RBIT instruction is used to reverse the bit order
>      of a word. This is present in ARMv6 and above in both
>      ARM and Thumb modes. This is also specified as an intrinsic
>      function in ACLE documentation.
>
>      This commit implements the GCC builtin for ARM target for
>      RBIT instruction, __builtin_arm_rbit. Also, this implements
>      the intrinsic functions as stated in ARM ACLE documentation,
>      which are listed below:
>
>      uint32_t __rbit(uint32_t x);
>      unsigned long __rbitl(unsigned long x);
>      uint64_t __rbitll(uint64_t x);
>
>      Note: __rbitll is implemented as two calls to __rbit. I know
>      this is not how it's done in AArch64, but this is what I can
>      do for now.
>
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index ae582172ab9..83dcb7b411c 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -11568,6 +11568,13 @@
>     [(set_attr "predicable" "yes")
>      (set_attr "type" "clz")])
>
> +(define_insn "rbit"
> +  [(set (match_operand:SI 0 "s_register_operand" "=r")
> +       (unspec:SI [(match_operand:SI 1 "s_register_operand" "r")] 
> UNSPEC_RBIT))]
> +  "TARGET_32BIT && arm_arch_thumb2"
> +  "rbit%?\\t%0, %1"
> +  [(set_attr "predicable" "yes")])
> +
>   (define_insn "rbitsi2"
>     [(set (match_operand:SI 0 "s_register_operand" "=r")
>          (unspec:SI [(match_operand:SI 1 "s_register_operand" "r")] 
> UNSPEC_RBIT))]
> diff --git a/gcc/config/arm/arm_acle.h b/gcc/config/arm/arm_acle.h
> index 2c7acc698ea..ce1b102444b 100644
> --- a/gcc/config/arm/arm_acle.h
> +++ b/gcc/config/arm/arm_acle.h
> @@ -168,6 +168,29 @@ __arm_mrrc2 (const unsigned int __coproc, const
> unsigned int __opc1,
>   {
>     return __builtin_arm_mrrc2 (__coproc, __opc1,  __CRm);
>   }
> +
> +__extension__ static __inline uint32_t __attribute__ 
> ((__always_inline__))
> +__rbit(uint32_t __op1)
> +{
> +  return __builtin_arm_rbit(__op1);
> +}
> +
> +__extension__ static __inline uint64_t __attribute__ 
> ((__always_inline__))
> +__rbitll(uint64_t __op1)
> +{
> +  return (((uint64_t)__rbit(__op1)) << 32U) | __rbit(__op1 >> 32U);
> +}
> +
> +__extension__ static __inline unsigned long __attribute__
> ((__always_inline__))
> +__rbitl(unsigned long __op1)
> +{
> +#if __SIZEOF_LONG__ == 4
> +  return __rbit(__op1);
> +#else
> +  return __rbitll(__op1);
> +#endif
> +}
> +
>   #endif /* __ARM_ARCH >= 6.  */
>   #endif /* __ARM_ARCH >= 6 ||  defined (__ARM_ARCH_5TE__).  */
>   #endif /*  __ARM_ARCH >= 5.  */
> diff --git a/gcc/config/arm/arm_acle_builtins.def
> b/gcc/config/arm/arm_acle_builtins.def
> index b2438d66da2..ecb3be491fc 100644
> --- a/gcc/config/arm/arm_acle_builtins.def
> +++ b/gcc/config/arm/arm_acle_builtins.def
> @@ -24,6 +24,7 @@ VAR1 (UBINOP, crc32w, si)
>   VAR1 (UBINOP, crc32cb, si)
>   VAR1 (UBINOP, crc32ch, si)
>   VAR1 (UBINOP, crc32cw, si)
> +VAR1 (UBINOP, rbit, si)
>   VAR1 (CDP, cdp, void)
>   VAR1 (CDP, cdp2, void)
>   VAR1 (LDC, ldc, void)
> diff --git a/gcc/testsuite/gcc.target/arm/acle/rbit.c
> b/gcc/testsuite/gcc.target/arm/acle/rbit.c
> new file mode 100644
> index 00000000000..7803dd33615
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/acle/rbit.c
> @@ -0,0 +1,18 @@
> +/* Test the crc32d ACLE intrinsic.  */
> +
> +/* { dg-do assemble } */
> +/* { dg-require-effective-target arm_crc_ok } */
> +/* { dg-options "-save-temps -O0" } */
> +/* { dg-add-options arm_crc } */
> +
> +#include "arm_acle.h"
> +
> +void test_rbit (void)
> +{
> +  uint32_t out_uint32_t;
> +  uint32_t arg0_uint32_t;
> +
> +  out_uint32_t = __rbit (arg0_uint32_t);
> +}
> +
> +/* { dg-final { scan-assembler-times "rbit\t...?, ...?\n" 2 } } */

Re: [PATCH] Adding RBIT gcc builtin for ARM

[Wilco Dijkstra]

Hi Ayan,

Have you seen https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50481?

Adding support for a generic bitreverse builtin would be very useful
since LLVM already supports this.

Wilco