public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RISC-V: Remove masking third operand of rotate instructions
@ 2023-05-10 15:50 Jivan Hakobyan
  2023-05-17  5:14 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Jivan Hakobyan @ 2023-05-10 15:50 UTC (permalink / raw)
  To: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 1255 bytes --]

Rotate instructions do not need to mask the third operand.
For example  RV64 the following code:

unsigned long foo1(unsigned long rs1, unsigned long rs2)
{
    long shamt = rs2 & (64 - 1);
    return (rs1 << shamt) | (rs1 >> ((64 - shamt) & (64 - 1)));
}

Compiles to:
foo1:
        andi    a1,a1,63
        rol     a0,a0,a1
        ret

This patch removes unnecessary masking.
Besides, I have merged masking insns for shifts that were written before.


gcc/ChangeLog:
        * config/riscv/riscv.md: Merged
        * config/riscv/bitmanip.md: New insns
        * config/riscv/iterators.md: New iterator and optab items
        * config/riscv/predicates.md: New predicates

gcc/testsuite/ChangeLog:
        * testsuite/gcc.target/riscv/shift-and-2.c: Fixed test
        * testsuite/gcc.target/riscv/zbb-rol-ror-01.c: New test
        * testsuite/gcc.target/riscv/zbb-rol-ror-02.c: New test
        * testsuite/gcc.target/riscv/zbb-rol-ror-03.c: New test
        * testsuite/gcc.target/riscv/zbb-rol-ror-04.c: New test
        * testsuite/gcc.target/riscv/zbb-rol-ror-05.c: New test
        * testsuite/gcc.target/riscv/zbb-rol-ror-06.c: New test
        * testsuite/gcc.target/riscv/zbb-rol-ror-07.c: New test


-- 
With the best regards
Jivan Hakobyan

[-- Attachment #2: rotate_mask.patch --]
[-- Type: text/x-patch, Size: 13422 bytes --]

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index a27fc3e34a1..0fd0cbdeb04 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -351,6 +351,42 @@
   "rolw\t%0,%1,%2"
   [(set_attr "type" "bitmanip")])
 
+(define_insn_and_split "*<bitmanip_optab><mode>3_mask"
+  [(set (match_operand:X     0 "register_operand" "= r")
+        (bitmanip_rotate:X
+            (match_operand:X 1 "register_operand" "  r")
+            (match_operator 4 "subreg_lowpart_operator"
+             [(and:X
+               (match_operand:X 2 "register_operand"  "r")
+               (match_operand 3 "<X:shiftm1>" "<X:shiftm1p>"))])))]
+  "TARGET_ZBB || TARGET_ZBKB"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+        (bitmanip_rotate:X (match_dup 1)
+                           (match_dup 2)))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "bitmanip")
+   (set_attr "mode" "<X:MODE>")])
+
+(define_insn_and_split "*<bitmanip_optab>si3_sext_mask"
+  [(set (match_operand:DI     0 "register_operand" "= r")
+  (sign_extend:DI (bitmanip_rotate:SI
+            (match_operand:SI 1 "register_operand" "  r")
+            (match_operator 4 "subreg_lowpart_operator"
+             [(and:DI
+               (match_operand:DI 2 "register_operand"  "r")
+               (match_operand 3 "const_si_mask_operand"))]))))]
+  "TARGET_64BIT && (TARGET_ZBB || TARGET_ZBKB)"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+  (sign_extend:DI (bitmanip_rotate:SI (match_dup 1)
+                           (match_dup 2))))]
+  "operands[2] = gen_lowpart (QImode, operands[2]);"
+  [(set_attr "type" "bitmanip")
+   (set_attr "mode" "DI")])
+
 ;; orc.b (or-combine) is added as an unspec for the benefit of the support
 ;; for optimized string functions (such as strcmp).
 (define_insn "orcb<mode>2"
diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
index 1d56324df03..8afe98e4410 100644
--- a/gcc/config/riscv/iterators.md
+++ b/gcc/config/riscv/iterators.md
@@ -117,7 +117,7 @@
 (define_mode_attr HALFMODE [(DF "SI") (DI "SI") (TF "DI")])
 
 ; bitmanip mode attribute
-(define_mode_attr shiftm1 [(SI "const31_operand") (DI "const63_operand")])
+(define_mode_attr shiftm1 [(SI "const_si_mask_operand") (DI "const_di_mask_operand")])
 (define_mode_attr shiftm1p [(SI "DsS") (DI "DsD")])
 
 ;; -------------------------------------------------------------------
@@ -174,6 +174,8 @@
 
 (define_code_iterator clz_ctz_pcnt [clz ctz popcount])
 
+(define_code_iterator bitmanip_rotate [rotate rotatert])
+
 ;; -------------------------------------------------------------------
 ;; Code Attributes
 ;; -------------------------------------------------------------------
@@ -271,7 +273,9 @@
 				  (umax "umax")
 				  (clz "clz")
 				  (ctz "ctz")
-				  (popcount "popcount")])
+				  (popcount "popcount")
+				  (rotate "rotl")
+				  (rotatert "rotr")])
 (define_code_attr bitmanip_insn [(smin "min")
 				 (smax "max")
 				 (umin "minu")
diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
index e5adf06fa25..ffcbb9a7589 100644
--- a/gcc/config/riscv/predicates.md
+++ b/gcc/config/riscv/predicates.md
@@ -235,13 +235,15 @@
   (and (match_code "const_int")
        (match_test "SINGLE_BIT_MASK_OPERAND (~UINTVAL (op))")))
 
-(define_predicate "const31_operand"
+(define_predicate "const_si_mask_operand"
   (and (match_code "const_int")
-       (match_test "INTVAL (op) == 31")))
+       (match_test "(INTVAL (op) & (GET_MODE_BITSIZE (SImode) - 1))
+                    == GET_MODE_BITSIZE (SImode) - 1")))
 
-(define_predicate "const63_operand"
+(define_predicate "const_di_mask_operand"
   (and (match_code "const_int")
-       (match_test "INTVAL (op) == 63")))
+       (match_test "(INTVAL (op) & (GET_MODE_BITSIZE (DImode) - 1))
+                    == GET_MODE_BITSIZE (DImode) - 1")))
 
 (define_predicate "imm5_operand"
   (and (match_code "const_int")
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index c508ee3ad89..777d9468efa 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2010,44 +2010,23 @@
   [(set_attr "type" "shift")
    (set_attr "mode" "SI")])
 
-(define_insn_and_split "*<optab>si3_mask"
-  [(set (match_operand:SI     0 "register_operand" "= r")
-	(any_shift:SI
-	    (match_operand:SI 1 "register_operand" "  r")
+(define_insn_and_split "*<optab><mode>3_mask"
+  [(set (match_operand:X     0 "register_operand" "= r")
+	(any_shift:X
+	    (match_operand:X 1 "register_operand" "  r")
 	    (match_operator 4 "subreg_lowpart_operator"
 	     [(and:SI
 	       (match_operand:SI 2 "register_operand"  "r")
-	       (match_operand 3 "const_int_operand"))])))]
-  "(INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
-   == GET_MODE_BITSIZE (SImode)-1"
-  "#"
-  "&& 1"
-  [(set (match_dup 0)
-	(any_shift:SI (match_dup 1)
-		      (match_dup 2)))]
-  "operands[2] = gen_lowpart (QImode, operands[2]);"
-  [(set_attr "type" "shift")
-   (set_attr "mode" "SI")])
-
-(define_insn_and_split "*<optab>si3_mask_1"
-  [(set (match_operand:SI     0 "register_operand" "= r")
-	(any_shift:SI
-	    (match_operand:SI 1 "register_operand" "  r")
-	    (match_operator 4 "subreg_lowpart_operator"
-	     [(and:DI
-	       (match_operand:DI 2 "register_operand"  "r")
-	       (match_operand 3 "const_int_operand"))])))]
-  "TARGET_64BIT
-   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
-       == GET_MODE_BITSIZE (SImode)-1"
+	       (match_operand 3 "<X:shiftm1>"))])))]
+  ""
   "#"
   "&& 1"
   [(set (match_dup 0)
-	(any_shift:SI (match_dup 1)
+	(any_shift:X (match_dup 1)
 		      (match_dup 2)))]
   "operands[2] = gen_lowpart (QImode, operands[2]);"
   [(set_attr "type" "shift")
-   (set_attr "mode" "SI")])
+   (set_attr "mode" "<X:MODE>")])
 
 (define_insn "<optab>di3"
   [(set (match_operand:DI 0 "register_operand"     "= r")
@@ -2065,45 +2044,23 @@
   [(set_attr "type" "shift")
    (set_attr "mode" "DI")])
 
-(define_insn_and_split "*<optab>di3_mask"
-  [(set (match_operand:DI     0 "register_operand" "= r")
-	(any_shift:DI
-	    (match_operand:DI 1 "register_operand" "  r")
-	    (match_operator 4 "subreg_lowpart_operator"
-	     [(and:SI
-	       (match_operand:SI 2 "register_operand"  "r")
-	       (match_operand 3 "const_int_operand"))])))]
-  "TARGET_64BIT
-   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1))
-       == GET_MODE_BITSIZE (DImode)-1"
-  "#"
-  "&& 1"
-  [(set (match_dup 0)
-	(any_shift:DI (match_dup 1)
-		      (match_dup 2)))]
-  "operands[2] = gen_lowpart (QImode, operands[2]);"
-  [(set_attr "type" "shift")
-   (set_attr "mode" "DI")])
-
-(define_insn_and_split "*<optab>di3_mask_1"
-  [(set (match_operand:DI     0 "register_operand" "= r")
-	(any_shift:DI
-	    (match_operand:DI 1 "register_operand" "  r")
+(define_insn_and_split "*<optab><mode>3_mask_1"
+  [(set (match_operand:GPR     0 "register_operand" "= r")
+	(any_shift:GPR
+	    (match_operand:GPR 1 "register_operand" "  r")
 	    (match_operator 4 "subreg_lowpart_operator"
 	     [(and:DI
 	       (match_operand:DI 2 "register_operand"  "r")
-	       (match_operand 3 "const_int_operand"))])))]
-  "TARGET_64BIT
-   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1))
-       == GET_MODE_BITSIZE (DImode)-1"
+	       (match_operand 3 "<GPR:shiftm1>"))])))]
+  "TARGET_64BIT"
   "#"
   "&& 1"
   [(set (match_dup 0)
-	(any_shift:DI (match_dup 1)
+	(any_shift:GPR (match_dup 1)
 		      (match_dup 2)))]
   "operands[2] = gen_lowpart (QImode, operands[2]);"
   [(set_attr "type" "shift")
-   (set_attr "mode" "DI")])
+   (set_attr "mode" "<GPR:MODE>")])
 
 (define_insn "*<optab>si3_extend"
   [(set (match_operand:DI                   0 "register_operand" "= r")
@@ -2126,34 +2083,10 @@
 	    (any_shift:SI
 	     (match_operand:SI 1 "register_operand" "  r")
 	     (match_operator 4 "subreg_lowpart_operator"
-	      [(and:SI
-	        (match_operand:SI 2 "register_operand" " r")
-	        (match_operand 3 "const_int_operand"))]))))]
-  "TARGET_64BIT
-   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
-       == GET_MODE_BITSIZE (SImode)-1"
-  "#"
-  "&& 1"
-  [(set (match_dup 0)
-	(sign_extend:DI
-	 (any_shift:SI (match_dup 1)
-		       (match_dup 2))))]
-  "operands[2] = gen_lowpart (QImode, operands[2]);"
-  [(set_attr "type" "shift")
-   (set_attr "mode" "SI")])
-
-(define_insn_and_split "*<optab>si3_extend_mask_1"
-  [(set (match_operand:DI                   0 "register_operand" "= r")
-	(sign_extend:DI
-	    (any_shift:SI
-	     (match_operand:SI 1 "register_operand" "  r")
-	     (match_operator 4 "subreg_lowpart_operator"
-	      [(and:DI
-	        (match_operand:DI 2 "register_operand" " r")
-	        (match_operand 3 "const_int_operand"))]))))]
-  "TARGET_64BIT
-   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode)-1))
-       == GET_MODE_BITSIZE (SImode)-1"
+	      [(and:GPR
+	        (match_operand:GPR 2 "register_operand" " r")
+	        (match_operand 3 "const_si_mask_operand"))]))))]
+  "TARGET_64BIT"
   "#"
   "&& 1"
   [(set (match_dup 0)
diff --git a/gcc/testsuite/gcc.target/riscv/shift-and-2.c b/gcc/testsuite/gcc.target/riscv/shift-and-2.c
index 360d8417209..bc01e8ef992 100644
--- a/gcc/testsuite/gcc.target/riscv/shift-and-2.c
+++ b/gcc/testsuite/gcc.target/riscv/shift-and-2.c
@@ -11,10 +11,10 @@ sub2 (int i, long j)
 }
 
 /* Test for <optab>si3_extend_mask.  */
-unsigned long
-sub3 (int mask)
+int
+sub3 (short mask)
 {
-  return 1 << (mask & 0xff);
+  return 1 << ((int)mask & 0x1f);
 }
 
 /* Test for <optab>si3_extend_mask_1.  */
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-01.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-01.c
index 20c1b2856ef..0a5b5e12eb2 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-01.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-01.c
@@ -14,4 +14,5 @@ unsigned long foo2(unsigned long rs1, unsigned long rs2)
 }
 
 /* { dg-final { scan-assembler-times "rol" 2 } } */
-/* { dg-final { scan-assembler-times "ror" 2 } } */
\ No newline at end of file
+/* { dg-final { scan-assembler-times "ror" 2 } } */
+/* { dg-final { scan-assembler-not "and" } } */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-02.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-02.c
index 14196c11fb9..d0d58135809 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-02.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-02.c
@@ -14,4 +14,5 @@ unsigned int foo2(unsigned int rs1, unsigned int rs2)
 }
 
 /* { dg-final { scan-assembler-times "rol" 2 } } */
-/* { dg-final { scan-assembler-times "ror" 2 } } */
\ No newline at end of file
+/* { dg-final { scan-assembler-times "ror" 2 } } */
+/* { dg-final { scan-assembler-not {and} { target { no-opts "-O0" } } } } */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-03.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-03.c
index ed4685dc7ac..b44d7fe8920 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-03.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-03.c
@@ -15,4 +15,5 @@ unsigned int ror(unsigned int rs1, unsigned int rs2)
 }
 
 /* { dg-final { scan-assembler-times "rolw" 1 } } */
-/* { dg-final { scan-assembler-times "rorw" 1 } } */
\ No newline at end of file
+/* { dg-final { scan-assembler-times "rorw" 1 } } */
+/* { dg-final { scan-assembler-not "and" } } */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-04.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-04.c
index 08053484cb2..7ef4c29dd5b 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-04.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-04.c
@@ -2,6 +2,7 @@
 /* { dg-options "-march=rv64gc_zbb -mabi=lp64d -fno-lto -O2" } */
 /* { dg-skip-if "" { *-*-* } { "-g" } } */
 /* { dg-final { check-function-bodies "**" "" } } */
+/* { dg-final { scan-assembler-not "and" } } */
 
 /*
 **foo1:
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-05.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-05.c
index 85090b1b0fc..2108ccc3e77 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-05.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-05.c
@@ -2,6 +2,7 @@
 /* { dg-options "-march=rv32gc_zbb -mabi=ilp32 -fno-lto -O2" } */
 /* { dg-skip-if "" { *-*-* } { "-g" } } */
 /* { dg-final { check-function-bodies "**" "" } } */
+/* { dg-final { scan-assembler-not "and" } } */
 
 /*
 **foo1:
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-06.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-06.c
index 70b79abb6ed..8c0711d6f94 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-06.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-06.c
@@ -2,6 +2,7 @@
 /* { dg-options "-march=rv64gc_zbb -mabi=lp64d -fno-lto -O2" } */
 /* { dg-skip-if "" { *-*-* } { "-g" } } */
 /* { dg-final { check-function-bodies "**" "" } } */
+/* { dg-final { scan-assembler-not "and" } } */
 
 /*
 **foo1:
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-07.c b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-07.c
index 3b6ab385a85..bda3f0e474d 100644
--- a/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-07.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-rol-ror-07.c
@@ -2,6 +2,7 @@
 /* { dg-options "-march=rv64gc_zbb -mabi=lp64d -fno-lto -O2" } */
 /* { dg-skip-if "" { *-*-* } { "-g" } } */
 /* { dg-final { check-function-bodies "**" "" } } */
+/* { dg-final { scan-assembler-not "and" } } */
 
 /*
 **foo1:

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: RISC-V: Remove masking third operand of rotate instructions
  2023-05-10 15:50 RISC-V: Remove masking third operand of rotate instructions Jivan Hakobyan
@ 2023-05-17  5:14 ` Jeff Law
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Law @ 2023-05-17  5:14 UTC (permalink / raw)
  To: Jivan Hakobyan, gcc-patches



On 5/10/23 09:50, Jivan Hakobyan via Gcc-patches wrote:
> Subject:
> RISC-V: Remove masking third operand of rotate instructions
> From:
> Jivan Hakobyan via Gcc-patches <gcc-patches@gcc.gnu.org>
> Date:
> 5/10/23, 09:50
> 
> To:
> gcc-patches@gcc.gnu.org
> 
> 
> Rotate instructions do not need to mask the third operand.
> For example  RV64 the following code:
> 
> unsigned long foo1(unsigned long rs1, unsigned long rs2)
> {
>      long shamt = rs2 & (64 - 1);
>      return (rs1 << shamt) | (rs1 >> ((64 - shamt) & (64 - 1)));
> }
> 
> Compiles to:
> foo1:
>          andi    a1,a1,63
>          rol     a0,a0,a1
>          ret
> 
> This patch removes unnecessary masking.
> Besides, I have merged masking insns for shifts that were written before.
> 
> 
> gcc/ChangeLog:
>          * config/riscv/riscv.md: Merged
>          * config/riscv/bitmanip.md: New insns
>          * config/riscv/iterators.md: New iterator and optab items
>          * config/riscv/predicates.md: New predicates
Usually we try to mention the patterns that got changed.  So something 
like this

	* config/riscv/riscv.md (*<optab><mode>3_mask):  New pattern,
	combined from....
	(*<optab>si3_mask, *<optab>di3_mask): Here.

Similarly for the other patterns in riscv.md that you combined.

For the bitmanip ChangeLog it might look like

	* config/riscv/bitmanip.md (*<bitmanip_optab><mode>3_mask): New
	pattern.
	(*<bitmanip_optab>si3_sext_mask): Likewise.


	* config/riscv/iterators.md (shiftm1): Generalize to handle more
	masking constants.
	(bitmanip_rotate): New iterator.
	(bitmanip_optab): Add rotates.

	* config/riscv/predicates.md (const_si_mask_operand): Renamed
	from const31_operand.  Generalize to handle more mask constants.
	(const_di_mask_operand): Similarly.


> 
> 
> -- With the best regards Jivan Hakobyan
> 
> 
> rotate_mask.patch
> 
> diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> index a27fc3e34a1..0fd0cbdeb04 100644
> --- a/gcc/config/riscv/bitmanip.md
> +++ b/gcc/config/riscv/bitmanip.md
> @@ -351,6 +351,42 @@
>     "rolw\t%0,%1,%2"
>     [(set_attr "type" "bitmanip")])
>   
> +(define_insn_and_split "*<bitmanip_optab><mode>3_mask"
> +  [(set (match_operand:X     0 "register_operand" "= r")
> +        (bitmanip_rotate:X
> +            (match_operand:X 1 "register_operand" "  r")
> +            (match_operator 4 "subreg_lowpart_operator"
> +             [(and:X
> +               (match_operand:X 2 "register_operand"  "r")
> +               (match_operand 3 "<X:shiftm1>" "<X:shiftm1p>"))])))]
> +  "TARGET_ZBB || TARGET_ZBKB"
> +  "#"
> +  "&& 1"
> +  [(set (match_dup 0)
> +        (bitmanip_rotate:X (match_dup 1)
> +                           (match_dup 2)))]
> +  "operands[2] = gen_lowpart (QImode, operands[2]);"
> +  [(set_attr "type" "bitmanip")
> +   (set_attr "mode" "<X:MODE>")])
It's worth noting that by using a subreg_lowpart_operator in this manner 
we can match any narrowing subreg lowpart rather than being restricted 
to QImode.  Clever use of iterators for the predicate and constraints on 
operand 3 as well.






> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index c508ee3ad89..777d9468efa 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -2010,44 +2010,23 @@
>     [(set_attr "type" "shift")
>      (set_attr "mode" "SI")])
>   
> -(define_insn_and_split "*<optab>si3_mask"
> -  [(set (match_operand:SI     0 "register_operand" "= r")
> -	(any_shift:SI
> -	    (match_operand:SI 1 "register_operand" "  r")
> +(define_insn_and_split "*<optab><mode>3_mask"
> +  [(set (match_operand:X     0 "register_operand" "= r")
> +	(any_shift:X
> +	    (match_operand:X 1 "register_operand" "  r")
>   	    (match_operator 4 "subreg_lowpart_operator"
>   	     [(and:SI
>   	       (match_operand:SI 2 "register_operand"  "r")
> -	       (match_operand 3 "const_int_operand"))])))]
Shouldn't the mode of operand 2 change to "X" as well?





> -(define_insn_and_split "*<optab>di3_mask_1"
> -  [(set (match_operand:DI     0 "register_operand" "= r")
> -	(any_shift:DI
> -	    (match_operand:DI 1 "register_operand" "  r")
> +(define_insn_and_split "*<optab><mode>3_mask_1"
> +  [(set (match_operand:GPR     0 "register_operand" "= r")
> +	(any_shift:GPR
> +	    (match_operand:GPR 1 "register_operand" "  r")
>   	    (match_operator 4 "subreg_lowpart_operator"
>   	     [(and:DI
>   	       (match_operand:DI 2 "register_operand"  "r")
> -	       (match_operand 3 "const_int_operand"))])))]
Presumably we use GPR here because for TARGET_64BIT we can match both 
the 32bit and 64bit opcodes?  I was wondering if we should do the same 
for the rotate patterns -- use the GPR iterator rather than the X 
iterator to match rol[w] and ror[w].

Overall it looks really good.  Both in terms of improving code 
generation for the rotates and cleaning up the shift patterns a bit too. 
  Just a couple questions/cleanups and an improved ChangeLog and this 
should be good to go.

jeff





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: RISC-V: Remove masking third operand of rotate instructions
@ 2023-05-18 15:37 Joern Rennecke
  0 siblings, 0 replies; 3+ messages in thread
From: Joern Rennecke @ 2023-05-18 15:37 UTC (permalink / raw)
  To: GCC Patches, Jivan Hakobyan; +Cc: Jeff Law

[-- Attachment #1: Type: text/plain, Size: 4738 bytes --]

This breaks building libstdc++-v3 for
-march=rv32imafdcv_zicsr_zifencei_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvl128b_zvl32b_zvl64b
-mabi=ilp32f .

[amylaar@damia tstd]$
/home/amylaar/embecosm/fsf-riscv/riscv-gnu-toolchain/build-gcc-newlib-stage2/./gcc/cc1plus
20230518-1.cc -quiet -dumpbase memory_resource.cc -dumpbase-ext .cc
-mcmodel=medany -march=rv32imafdcv_zicsr
_zifencei_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvl128b_zvl32b_zvl64b
-mabi=ilp32f -mtune=rocket -misa-spec=20191213
-march=rv32imafdcv_zicsr_zifencei_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvl128
b_zvl32b_zvl64b -Os -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2
-std=gnu++17 -version -fdiagnostics-show-location=once
-ffunction-sections -fdata-sections -frandom-seed=memory_resource.lo
-fimplicit-templates -o mem
ory_resource.s
GNU C++17 (GCC) version 14.0.0 20230518 (experimental) (riscv64-unknown-elf)
        compiled by GNU C version 11.3.1 20220421 (Red Hat 11.3.1-2),
GMP version 6.2.0, MPFR version 4.1.0-p13, MPC version 1.2.1, isl
version isl-0.16.1-GMP

GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: d7da457b477a50631d59e9ee36a0d79f
/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc:226:36:
warning: 'std::pmr::monotonic_buffer_resource::_Chunk' has a field
'std::pmr::{anonymous}::aligned_size<64> std::pmr::monotonic_buffer
_resource::_Chunk::_M_size' whose type uses the anonymous namespace
[-Wsubobject-linkage]
  226 |   class monotonic_buffer_resource::_Chunk
      |                                    ^~~~~~
/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc:756:12:
warning: 'std::pmr::__pool_resource::_Pool::vector' has a field
'std::pmr::{anonymous}::chunk* std::pmr::__pool_resource::_Pool::vecto
r::data' whose type uses the anonymous namespace [-Wsubobject-linkage]
  756 |     struct vector
      |            ^~~~~~
/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc:861:27:
warning: 'std::pmr::__pool_resource::_BigBlock' has a base
'std::pmr::{anonymous}::big_block' which uses the anonymous namespace
[-Wsu
bobject-linkage]
  861 |   struct __pool_resource::_BigBlock : big_block
      |                           ^~~~~~~~~
/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc:
In member function 'void
std::pmr::monotonic_buffer_resource::_M_release_buffers()':
/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc:302:3:
error: unable to generate reloads for:
  302 |   }
      |   ^
(insn 22 21 23 4 (set (reg:SI 13 a3)
        (ashift:SI (const_int 1 [0x1])
            (subreg:QI (and:SI (reg:SI 137 [ _9 ])
                    (const_int 63 [0x3f])) 0)))
"/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc":274:19
403 {*bsetsi_1_mask}
     (expr_list:REG_DEAD (reg:SI 137 [ _9 ])
        (nil)))
during RTL pass: reload
/home/amylaar/embecosm/fsf-riscv/gcc/libstdc++-v3/src/c++17/memory_resource.cc:302:3:
internal compiler error: in curr_insn_transform, at
lra-constraints.cc:4231
0x8a3219 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
        /home/amylaar/embecosm/fsf-riscv/gcc/gcc/rtl-error.cc:108
0x87c243 curr_insn_transform
        /home/amylaar/embecosm/fsf-riscv/gcc/gcc/lra-constraints.cc:4231
0x10ab673 lra_constraints(bool)
        /home/amylaar/embecosm/fsf-riscv/gcc/gcc/lra-constraints.cc:5396
0x1093092 lra(_IO_FILE*)
        /home/amylaar/embecosm/fsf-riscv/gcc/gcc/lra.cc:2375
0x104bd41 do_reload
        /home/amylaar/embecosm/fsf-riscv/gcc/gcc/ira.cc:5967
0x104bd41 execute
        /home/amylaar/embecosm/fsf-riscv/gcc/gcc/ira.cc:6153
Please submit a full bug report, with preprocessed source (by using
-freport-bug).

The issue is that you have loosened the <X:shiftm1> predicates used in
 bset<X:mode>_1_mask without likewise loosening the <X:shiftm1p> / DsS
/ DsD constraints.

The attached patch fixes this build failure.

To reproduce the failure compile the attached 20230518-1.cc with the
riscv cc1plus
<path-to-your-gcc-build-directory>/cc1plus 20230518-1.cc -quiet
-dumpbase memory_resource.cc -dumpbase-ext .cc -mcmodel=medany
-march=rv32imafdcv_zicsr_zifencei_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvl128b_zvl32b_zvl64b
-mabi=ilp32f -mtune=rocket -misa-spec=20191213
-march=rv32imafdcv_zicsr_zifencei_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvl128b_zvl32b_zvl64b
-Os -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2 -std=gnu++17
-version -fdiagnostics-show-location=once -ffunction-sections
-fdata-sections -frandom-seed=memory_resource.lo -fimplicit-templates
-o memory_resource.s

[-- Attachment #2: riscv-DsS-patch.txt --]
[-- Type: text/plain, Size: 572 bytes --]

diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
index c448e6b37e9..44525b2da49 100644
--- a/gcc/config/riscv/constraints.md
+++ b/gcc/config/riscv/constraints.md
@@ -65,13 +65,13 @@
   "@internal
    31 immediate"
   (and (match_code "const_int")
-       (match_test "ival == 31")))
+       (match_test "(ival & 31) == 31")))
 
 (define_constraint "DsD"
   "@internal
    63 immediate"
   (and (match_code "const_int")
-       (match_test "ival == 63")))
+       (match_test "(ival & 63) == 63")))
 
 (define_constraint "DbS"
   "@internal"

[-- Attachment #3: 20230518-1.cc.gz --]
[-- Type: application/gzip, Size: 135021 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-05-18 15:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 15:50 RISC-V: Remove masking third operand of rotate instructions Jivan Hakobyan
2023-05-17  5:14 ` Jeff Law
2023-05-18 15:37 Joern Rennecke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).