diff mbox series

LoongArch: Rework bswap{hi,si,di}2 definition

Message ID 20240729075837.6060-1-xry111@xry111.site
State New
Headers show
Series LoongArch: Rework bswap{hi,si,di}2 definition | expand

Commit Message

Xi Ruoyao July 29, 2024, 7:58 a.m. UTC
Per a gcc-help thread we are generating sub-optimal code for
__builtin_bswap{32,64}.  To fix it:

- Use a single revb.d instruction for bswapdi2.
- Use a single revb.2w instruction for bswapsi2 for TARGET_64BIT,
  revb.2h + rotri.w for !TARGET_64BIT.
- Use a single revb.2h instruction for bswapsi2 (x) r>> 16, and a single
  revb.2w instruction for bswapdi2 (x) r>> 32.

Unfortunately I cannot figure out a way to make the compiler generate
revb.4h or revh.{2w,d} instructions.

gcc/ChangeLog:

	* config/loongarch/loongarch.md (UNSPEC_REVB_2H, UNSPEC_REVB_4H,
	UNSPEC_REVH_D): Remove UNSPECs.
	(revb_4h, revh_d): Remove define_insn.
	(revb_2h): Define as (rotatert:SI (bswap:SI x) 16) instead of
	an UNSPEC.
	(revb_2h_extend, revb_2w, *bswapsi2, bswapdi2): New define_insn.
	(bswapsi2): Change to define_expand.  Only expand to revb.2h +
	rotri.w if !TARGET_64BIT.
	(bswapdi2): Change to define_insn of which the output is just a
	revb.d instruction.

gcc/testsuite/ChangeLog:

	* gcc.target/loongarch/revb.c: New test.
---

Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?

 gcc/config/loongarch/loongarch.md         | 79 ++++++++++++-----------
 gcc/testsuite/gcc.target/loongarch/revb.c | 61 +++++++++++++++++
 2 files changed, 104 insertions(+), 36 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/revb.c

Comments

Lulu Cheng July 31, 2024, 8:57 a.m. UTC | #1
在 2024/7/29 下午3:58, Xi Ruoyao 写道:
> Per a gcc-help thread we are generating sub-optimal code for
> __builtin_bswap{32,64}.  To fix it:
>
> - Use a single revb.d instruction for bswapdi2.
> - Use a single revb.2w instruction for bswapsi2 for TARGET_64BIT,
>    revb.2h + rotri.w for !TARGET_64BIT.
> - Use a single revb.2h instruction for bswapsi2 (x) r>> 16, and a single
>    revb.2w instruction for bswapdi2 (x) r>> 32.
>
> Unfortunately I cannot figure out a way to make the compiler generate
> revb.4h or revh.{2w,d} instructions.

This optimization is really ingenious and I have no problem.

I also haven't figured out how to generate revb.4h or revh. {2w,d}.
I think we can merge this patch first.

Thanks.

>
> gcc/ChangeLog:
>
> 	* config/loongarch/loongarch.md (UNSPEC_REVB_2H, UNSPEC_REVB_4H,
> 	UNSPEC_REVH_D): Remove UNSPECs.
> 	(revb_4h, revh_d): Remove define_insn.
> 	(revb_2h): Define as (rotatert:SI (bswap:SI x) 16) instead of
> 	an UNSPEC.
> 	(revb_2h_extend, revb_2w, *bswapsi2, bswapdi2): New define_insn.
> 	(bswapsi2): Change to define_expand.  Only expand to revb.2h +
> 	rotri.w if !TARGET_64BIT.
> 	(bswapdi2): Change to define_insn of which the output is just a
> 	revb.d instruction.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/loongarch/revb.c: New test.
> ---
>
> Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?
>
>   gcc/config/loongarch/loongarch.md         | 79 ++++++++++++-----------
>   gcc/testsuite/gcc.target/loongarch/revb.c | 61 +++++++++++++++++
>   2 files changed, 104 insertions(+), 36 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/loongarch/revb.c
>
> diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md
> index ac94a22eafc..f166e834c56 100644
> --- a/gcc/config/loongarch/loongarch.md
> +++ b/gcc/config/loongarch/loongarch.md
> @@ -20,11 +20,6 @@
>   ;; <http://www.gnu.org/licenses/>.
>   
>   (define_c_enum "unspec" [
> -  ;; Integer operations that are too cumbersome to describe directly.
> -  UNSPEC_REVB_2H
> -  UNSPEC_REVB_4H
> -  UNSPEC_REVH_D
> -
>     ;; Floating-point moves.
>     UNSPEC_LOAD_LOW
>     UNSPEC_LOAD_HIGH
> @@ -3155,55 +3150,67 @@ (define_insn "alslsi3_extend"
>   
>   ;; Reverse the order of bytes of operand 1 and store the result in operand 0.
>   
> -(define_insn "bswaphi2"
> -  [(set (match_operand:HI 0 "register_operand" "=r")
> -	(bswap:HI (match_operand:HI 1 "register_operand" "r")))]
> +(define_insn "revb_2h"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +	(rotatert:SI (bswap:SI (match_operand:SI 1 "register_operand" "r"))
> +		     (const_int 16)))]
>     ""
>     "revb.2h\t%0,%1"
>     [(set_attr "type" "shift")])
>   
> -(define_insn_and_split "bswapsi2"
> -  [(set (match_operand:SI 0 "register_operand" "=r")
> -	(bswap:SI (match_operand:SI 1 "register_operand" "r")))]
> -  ""
> -  "#"
> -  ""
> -  [(set (match_dup 0) (unspec:SI [(match_dup 1)] UNSPEC_REVB_2H))
> -   (set (match_dup 0) (rotatert:SI (match_dup 0) (const_int 16)))]
> -  ""
> -  [(set_attr "insn_count" "2")])
> -
> -(define_insn_and_split "bswapdi2"
> +(define_insn "revb_2h_extend"
>     [(set (match_operand:DI 0 "register_operand" "=r")
> -	(bswap:DI (match_operand:DI 1 "register_operand" "r")))]
> +	(sign_extend:DI
> +	  (rotatert:SI
> +	    (bswap:SI (match_operand:SI 1 "register_operand" "r"))
> +	    (const_int 16))))]
>     "TARGET_64BIT"
> -  "#"
> -  ""
> -  [(set (match_dup 0) (unspec:DI [(match_dup 1)] UNSPEC_REVB_4H))
> -   (set (match_dup 0) (unspec:DI [(match_dup 0)] UNSPEC_REVH_D))]
> -  ""
> -  [(set_attr "insn_count" "2")])
> +  "revb.2h\t%0,%1"
> +  [(set_attr "type" "shift")])
>   
> -(define_insn "revb_2h"
> -  [(set (match_operand:SI 0 "register_operand" "=r")
> -	(unspec:SI [(match_operand:SI 1 "register_operand" "r")] UNSPEC_REVB_2H))]
> +(define_insn "bswaphi2"
> +  [(set (match_operand:HI 0 "register_operand" "=r")
> +	(bswap:HI (match_operand:HI 1 "register_operand" "r")))]
>     ""
>     "revb.2h\t%0,%1"
>     [(set_attr "type" "shift")])
>   
> -(define_insn "revb_4h"
> +(define_insn "revb_2w"
>     [(set (match_operand:DI 0 "register_operand" "=r")
> -	(unspec:DI [(match_operand:DI 1 "register_operand" "r")] UNSPEC_REVB_4H))]
> +	(rotatert:DI (bswap:DI (match_operand:DI 1 "register_operand" "r"))
> +		     (const_int 32)))]
>     "TARGET_64BIT"
> -  "revb.4h\t%0,%1"
> +  "revb.2w\t%0,%1"
>     [(set_attr "type" "shift")])
>   
> -(define_insn "revh_d"
> +(define_insn "*bswapsi2"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +	(bswap:SI (match_operand:SI 1 "register_operand" "r")))]
> +  "TARGET_64BIT"
> +  "revb.2w\t%0,%1"
> +  [(set_attr "type" "shift")])
> +
> +(define_expand "bswapsi2"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> +	(bswap:SI (match_operand:SI 1 "register_operand" "r")))]
> +  ""
> +{
> +  if (!TARGET_64BIT)
> +    {
> +      rtx t = gen_reg_rtx (SImode);
> +      emit_insn (gen_revb_2h (t, operands[1]));
> +      emit_insn (gen_rotrsi3 (operands[0], t, GEN_INT (16)));
> +      DONE;
> +    }
> +})
> +
> +(define_insn "bswapdi2"
>     [(set (match_operand:DI 0 "register_operand" "=r")
> -	(unspec:DI [(match_operand:DI 1 "register_operand" "r")] UNSPEC_REVH_D))]
> +	(bswap:DI (match_operand:DI 1 "register_operand" "r")))]
>     "TARGET_64BIT"
> -  "revh.d\t%0,%1"
> +  "revb.d\t%0,%1"
>     [(set_attr "type" "shift")])
> +
>   
>   ;;
>   ;;  ....................
> diff --git a/gcc/testsuite/gcc.target/loongarch/revb.c b/gcc/testsuite/gcc.target/loongarch/revb.c
> new file mode 100644
> index 00000000000..27a5d0fc7b7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/loongarch/revb.c
> @@ -0,0 +1,61 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -march=loongarch64 -mabi=lp64d" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +**t1:
> +**	revb.2w	\$r4,\$r4
> +**	slli.w	\$r4,\$r4,0
> +**	jr	\$r1
> +*/
> +unsigned int
> +t1 (unsigned int x)
> +{
> +  return __builtin_bswap32 (x);
> +}
> +
> +/*
> +**t2:
> +**	revb.d	\$r4,\$r4
> +**	jr	\$r1
> +*/
> +unsigned long
> +t2 (unsigned long x)
> +{
> +  return __builtin_bswap64 (x);
> +}
> +
> +/*
> +**t3:
> +**	revb.2h	\$r4,\$r4
> +**	jr	\$r1
> +*/
> +unsigned int
> +t3 (unsigned int x)
> +{
> +  return (x >> 8) & 0xff00ff | (x << 8) & 0xff00ff00;
> +}
> +
> +/*
> +**t4:
> +**	revb.2w	\$r4,\$r4
> +**	jr	\$r1
> +*/
> +unsigned long
> +t4 (unsigned long x)
> +{
> +  x = __builtin_bswap64 (x);
> +  return x << 32 | x >> 32;
> +}
> +
> +/*
> +**t5:
> +**	revb.2h	\$r4,\$r4
> +**	bstrpick.w	\$r4,\$r4,15,0
> +**	jr	\$r1
> +*/
> +unsigned short
> +t5 (unsigned short x)
> +{
> +  return __builtin_bswap16 (x);
> +}
Xi Ruoyao July 31, 2024, 10:25 a.m. UTC | #2
On Wed, 2024-07-31 at 16:57 +0800, Lulu Cheng wrote:
> 
> 在 2024/7/29 下午3:58, Xi Ruoyao 写道:
> > Per a gcc-help thread we are generating sub-optimal code for
> > __builtin_bswap{32,64}.  To fix it:
> > 
> > - Use a single revb.d instruction for bswapdi2.
> > - Use a single revb.2w instruction for bswapsi2 for TARGET_64BIT,
> >     revb.2h + rotri.w for !TARGET_64BIT.
> > - Use a single revb.2h instruction for bswapsi2 (x) r>> 16, and a single
> >     revb.2w instruction for bswapdi2 (x) r>> 32.
> > 
> > Unfortunately I cannot figure out a way to make the compiler generate
> > revb.4h or revh.{2w,d} instructions.
> 
> This optimization is really ingenious and I have no problem.
> 
> I also haven't figured out how to generate revb.4h or revh. {2w,d}.
> I think we can merge this patch first.

Pushed r15-2433.

FWIW I tried a naive pattern for revh.2w:

(set (match_operand:DI 0 "register_operand" "=r")
     (ior:DI
       (and:DI
         (ashift:DI (match_operand:DI 1 "register_operand" "r")
                    (const_int 16))
         (const_int 18446462603027742720))
       (and:DI
         (lshiftrt:DI (match_dup 1)
                      (const_int 16))
         (const_int 281470681808895))))

But it seems too complex to be recognized.
Lulu Cheng Aug. 1, 2024, 1:08 a.m. UTC | #3
在 2024/7/31 下午6:25, Xi Ruoyao 写道:
> On Wed, 2024-07-31 at 16:57 +0800, Lulu Cheng wrote:
>> 在 2024/7/29 下午3:58, Xi Ruoyao 写道:
>>> Per a gcc-help thread we are generating sub-optimal code for
>>> __builtin_bswap{32,64}.  To fix it:
>>>
>>> - Use a single revb.d instruction for bswapdi2.
>>> - Use a single revb.2w instruction for bswapsi2 for TARGET_64BIT,
>>>      revb.2h + rotri.w for !TARGET_64BIT.
>>> - Use a single revb.2h instruction for bswapsi2 (x) r>> 16, and a single
>>>      revb.2w instruction for bswapdi2 (x) r>> 32.
>>>
>>> Unfortunately I cannot figure out a way to make the compiler generate
>>> revb.4h or revh.{2w,d} instructions.
>> This optimization is really ingenious and I have no problem.
>>
>> I also haven't figured out how to generate revb.4h or revh. {2w,d}.
>> I think we can merge this patch first.
> Pushed r15-2433.
Ok. Thanks!
>
> FWIW I tried a naive pattern for revh.2w:
>
> (set (match_operand:DI 0 "register_operand" "=r")
>       (ior:DI
>         (and:DI
>           (ashift:DI (match_operand:DI 1 "register_operand" "r")
>                      (const_int 16))
>           (const_int 18446462603027742720))
>         (and:DI
>           (lshiftrt:DI (match_dup 1)
>                        (const_int 16))
>           (const_int 281470681808895))))
> But it seems too complex to be recognized.

I think it needs to be recognized as a bswap operation in the tree-bswap 
phase,

but it seems a bit difficult to be recognized

>
diff mbox series

Patch

diff --git a/gcc/config/loongarch/loongarch.md b/gcc/config/loongarch/loongarch.md
index ac94a22eafc..f166e834c56 100644
--- a/gcc/config/loongarch/loongarch.md
+++ b/gcc/config/loongarch/loongarch.md
@@ -20,11 +20,6 @@ 
 ;; <http://www.gnu.org/licenses/>.
 
 (define_c_enum "unspec" [
-  ;; Integer operations that are too cumbersome to describe directly.
-  UNSPEC_REVB_2H
-  UNSPEC_REVB_4H
-  UNSPEC_REVH_D
-
   ;; Floating-point moves.
   UNSPEC_LOAD_LOW
   UNSPEC_LOAD_HIGH
@@ -3155,55 +3150,67 @@  (define_insn "alslsi3_extend"
 
 ;; Reverse the order of bytes of operand 1 and store the result in operand 0.
 
-(define_insn "bswaphi2"
-  [(set (match_operand:HI 0 "register_operand" "=r")
-	(bswap:HI (match_operand:HI 1 "register_operand" "r")))]
+(define_insn "revb_2h"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(rotatert:SI (bswap:SI (match_operand:SI 1 "register_operand" "r"))
+		     (const_int 16)))]
   ""
   "revb.2h\t%0,%1"
   [(set_attr "type" "shift")])
 
-(define_insn_and_split "bswapsi2"
-  [(set (match_operand:SI 0 "register_operand" "=r")
-	(bswap:SI (match_operand:SI 1 "register_operand" "r")))]
-  ""
-  "#"
-  ""
-  [(set (match_dup 0) (unspec:SI [(match_dup 1)] UNSPEC_REVB_2H))
-   (set (match_dup 0) (rotatert:SI (match_dup 0) (const_int 16)))]
-  ""
-  [(set_attr "insn_count" "2")])
-
-(define_insn_and_split "bswapdi2"
+(define_insn "revb_2h_extend"
   [(set (match_operand:DI 0 "register_operand" "=r")
-	(bswap:DI (match_operand:DI 1 "register_operand" "r")))]
+	(sign_extend:DI
+	  (rotatert:SI
+	    (bswap:SI (match_operand:SI 1 "register_operand" "r"))
+	    (const_int 16))))]
   "TARGET_64BIT"
-  "#"
-  ""
-  [(set (match_dup 0) (unspec:DI [(match_dup 1)] UNSPEC_REVB_4H))
-   (set (match_dup 0) (unspec:DI [(match_dup 0)] UNSPEC_REVH_D))]
-  ""
-  [(set_attr "insn_count" "2")])
+  "revb.2h\t%0,%1"
+  [(set_attr "type" "shift")])
 
-(define_insn "revb_2h"
-  [(set (match_operand:SI 0 "register_operand" "=r")
-	(unspec:SI [(match_operand:SI 1 "register_operand" "r")] UNSPEC_REVB_2H))]
+(define_insn "bswaphi2"
+  [(set (match_operand:HI 0 "register_operand" "=r")
+	(bswap:HI (match_operand:HI 1 "register_operand" "r")))]
   ""
   "revb.2h\t%0,%1"
   [(set_attr "type" "shift")])
 
-(define_insn "revb_4h"
+(define_insn "revb_2w"
   [(set (match_operand:DI 0 "register_operand" "=r")
-	(unspec:DI [(match_operand:DI 1 "register_operand" "r")] UNSPEC_REVB_4H))]
+	(rotatert:DI (bswap:DI (match_operand:DI 1 "register_operand" "r"))
+		     (const_int 32)))]
   "TARGET_64BIT"
-  "revb.4h\t%0,%1"
+  "revb.2w\t%0,%1"
   [(set_attr "type" "shift")])
 
-(define_insn "revh_d"
+(define_insn "*bswapsi2"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(bswap:SI (match_operand:SI 1 "register_operand" "r")))]
+  "TARGET_64BIT"
+  "revb.2w\t%0,%1"
+  [(set_attr "type" "shift")])
+
+(define_expand "bswapsi2"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(bswap:SI (match_operand:SI 1 "register_operand" "r")))]
+  ""
+{
+  if (!TARGET_64BIT)
+    {
+      rtx t = gen_reg_rtx (SImode);
+      emit_insn (gen_revb_2h (t, operands[1]));
+      emit_insn (gen_rotrsi3 (operands[0], t, GEN_INT (16)));
+      DONE;
+    }
+})
+
+(define_insn "bswapdi2"
   [(set (match_operand:DI 0 "register_operand" "=r")
-	(unspec:DI [(match_operand:DI 1 "register_operand" "r")] UNSPEC_REVH_D))]
+	(bswap:DI (match_operand:DI 1 "register_operand" "r")))]
   "TARGET_64BIT"
-  "revh.d\t%0,%1"
+  "revb.d\t%0,%1"
   [(set_attr "type" "shift")])
+
 
 ;;
 ;;  ....................
diff --git a/gcc/testsuite/gcc.target/loongarch/revb.c b/gcc/testsuite/gcc.target/loongarch/revb.c
new file mode 100644
index 00000000000..27a5d0fc7b7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/revb.c
@@ -0,0 +1,61 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=loongarch64 -mabi=lp64d" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+**t1:
+**	revb.2w	\$r4,\$r4
+**	slli.w	\$r4,\$r4,0
+**	jr	\$r1
+*/
+unsigned int
+t1 (unsigned int x)
+{
+  return __builtin_bswap32 (x);
+}
+
+/*
+**t2:
+**	revb.d	\$r4,\$r4
+**	jr	\$r1
+*/
+unsigned long
+t2 (unsigned long x)
+{
+  return __builtin_bswap64 (x);
+}
+
+/*
+**t3:
+**	revb.2h	\$r4,\$r4
+**	jr	\$r1
+*/
+unsigned int
+t3 (unsigned int x)
+{
+  return (x >> 8) & 0xff00ff | (x << 8) & 0xff00ff00;
+}
+
+/*
+**t4:
+**	revb.2w	\$r4,\$r4
+**	jr	\$r1
+*/
+unsigned long
+t4 (unsigned long x)
+{
+  x = __builtin_bswap64 (x);
+  return x << 32 | x >> 32;
+}
+
+/*
+**t5:
+**	revb.2h	\$r4,\$r4
+**	bstrpick.w	\$r4,\$r4,15,0
+**	jr	\$r1
+*/
+unsigned short
+t5 (unsigned short x)
+{
+  return __builtin_bswap16 (x);
+}