[1/3] xtensa: Improve "*shlrd_reg" insn pattern and its variant

Message ID b5519bca-177d-a9e8-f1cb-de5a9eb90313@yahoo.co.jp
State New
Headers
Series [1/3] xtensa: Improve "*shlrd_reg" insn pattern and its variant |

Commit Message

Takayuki 'January June' Suwa May 30, 2023, 9:25 a.m. UTC
  The insn "*shlrd_reg" shifts two registers with a funnel shifter by the
third register to get a single word result:

  reg0 = (reg1 SHIFT_OP0 reg3) BIT_JOIN_OP (reg2 SHIFT_OP1 (32 - reg3))

where the funnel left shift is SHIFT_OP0 := ASHIFT, SHIFT_OP1 := LSHIFTRT
and its right shift is SHIFT_OP0 := LSHIFTRT, SHIFT_OP1 := ASHIFT,
respectively.  And also, BIT_JOIN_OP can be either PLUS or IOR in either
shift direction.

  [(set (match_operand:SI 0 "register_operand" "=a")
	(match_operator:SI 6 "xtensa_bit_join_operator"
		[(match_operator:SI 4 "logical_shift_operator"
			[(match_operand:SI 1 "register_operand" "r")
			 (match_operand:SI 3 "register_operand" "r")])
		 (match_operator:SI 5 "logical_shift_operator"
			[(match_operand:SI 2 "register_operand" "r")
			 (neg:SI (match_dup 3))])]))]

Although the RTL matching template can express it as above, there is no
way of direcing that the operator (operands[6]) that combines the two
individual shifts is commutative.
Thus, if multiple insn sequences matching the above pattern appear
adjacently, the combiner may accidentally mix them up and get partial
results.

This patch adds a new insn-and-split pattern with the two sides swapped
representation of the bit-combining operation that was lacking and
described above.

And also changes the other "*shlrd" variants from previously describing
the arbitraryness of bit-combining operations with code iterators to a
combination of the match_operator and the predicate above.

gcc/ChangeLog:

	* config/xtensa/predicates.md (xtensa_bit_join_operator):
	New predicate.
	* config/xtensa/xtensa.md (ior_op): Remove.
	(*shlrd_reg): Rename from "*shlrd_reg_<code>", and add the
	insn_and_split pattern of the same name to express and capture
	the bit-combining operation with both sides swapped.
	In addition, replace use of code iterator with new operator
	predicate.
	(*shlrd_const, *shlrd_per_byte):
	Likewise regarding the code iterator.
---
 gcc/config/xtensa/predicates.md |  3 ++
 gcc/config/xtensa/xtensa.md     | 81 ++++++++++++++++++++++-----------
 2 files changed, 58 insertions(+), 26 deletions(-)
  

Comments

Max Filippov May 31, 2023, 9:51 a.m. UTC | #1
On Tue, May 30, 2023 at 2:27 AM Takayuki 'January June' Suwa
<jjsuwa_sys3175@yahoo.co.jp> wrote:
>
> The insn "*shlrd_reg" shifts two registers with a funnel shifter by the
> third register to get a single word result:
>
>   reg0 = (reg1 SHIFT_OP0 reg3) BIT_JOIN_OP (reg2 SHIFT_OP1 (32 - reg3))
>
> where the funnel left shift is SHIFT_OP0 := ASHIFT, SHIFT_OP1 := LSHIFTRT
> and its right shift is SHIFT_OP0 := LSHIFTRT, SHIFT_OP1 := ASHIFT,
> respectively.  And also, BIT_JOIN_OP can be either PLUS or IOR in either
> shift direction.
>
>   [(set (match_operand:SI 0 "register_operand" "=a")
>         (match_operator:SI 6 "xtensa_bit_join_operator"
>                 [(match_operator:SI 4 "logical_shift_operator"
>                         [(match_operand:SI 1 "register_operand" "r")
>                          (match_operand:SI 3 "register_operand" "r")])
>                  (match_operator:SI 5 "logical_shift_operator"
>                         [(match_operand:SI 2 "register_operand" "r")
>                          (neg:SI (match_dup 3))])]))]
>
> Although the RTL matching template can express it as above, there is no
> way of direcing that the operator (operands[6]) that combines the two
> individual shifts is commutative.
> Thus, if multiple insn sequences matching the above pattern appear
> adjacently, the combiner may accidentally mix them up and get partial
> results.
>
> This patch adds a new insn-and-split pattern with the two sides swapped
> representation of the bit-combining operation that was lacking and
> described above.
>
> And also changes the other "*shlrd" variants from previously describing
> the arbitraryness of bit-combining operations with code iterators to a
> combination of the match_operator and the predicate above.
>
> gcc/ChangeLog:
>
>         * config/xtensa/predicates.md (xtensa_bit_join_operator):
>         New predicate.
>         * config/xtensa/xtensa.md (ior_op): Remove.
>         (*shlrd_reg): Rename from "*shlrd_reg_<code>", and add the
>         insn_and_split pattern of the same name to express and capture
>         the bit-combining operation with both sides swapped.
>         In addition, replace use of code iterator with new operator
>         predicate.
>         (*shlrd_const, *shlrd_per_byte):
>         Likewise regarding the code iterator.
> ---
>  gcc/config/xtensa/predicates.md |  3 ++
>  gcc/config/xtensa/xtensa.md     | 81 ++++++++++++++++++++++-----------
>  2 files changed, 58 insertions(+), 26 deletions(-)

Regtested for target=xtensa-linux-uclibc, no new regressions.
I can also confirm that the pattern is now used as expected in
the case where I previously had an issue. Thanks for fixing that!
Committed to master.
  

Patch

diff --git a/gcc/config/xtensa/predicates.md b/gcc/config/xtensa/predicates.md
index 5faf1be8c15..a3575a68892 100644
--- a/gcc/config/xtensa/predicates.md
+++ b/gcc/config/xtensa/predicates.md
@@ -200,6 +200,9 @@ 
 (define_predicate "xtensa_shift_per_byte_operator"
   (match_code "ashift,ashiftrt,lshiftrt"))
 
+(define_predicate "xtensa_bit_join_operator"
+  (match_code "plus,ior"))
+
 (define_predicate "tls_symbol_operand"
   (and (match_code "symbol_ref")
        (match_test "SYMBOL_REF_TLS_MODEL (op) != 0")))
diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
index 57e50911f52..eda1353894b 100644
--- a/gcc/config/xtensa/xtensa.md
+++ b/gcc/config/xtensa/xtensa.md
@@ -87,9 +87,6 @@ 
 ;; the same template.
 (define_mode_iterator HQI [HI QI])
 
-;; This code iterator is for *shlrd and its variants.
-(define_code_iterator ior_op [ior plus])
-
 
 ;; Attributes.
 
@@ -1682,21 +1679,22 @@ 
    (set_attr "mode"	"SI")
    (set_attr "length"	"9")])
 
-(define_insn "*shlrd_reg_<code>"
+(define_insn "*shlrd_reg"
   [(set (match_operand:SI 0 "register_operand" "=a")
-	(ior_op:SI (match_operator:SI 4 "logical_shift_operator"
+	(match_operator:SI 6 "xtensa_bit_join_operator"
+		[(match_operator:SI 4 "logical_shift_operator"
 			[(match_operand:SI 1 "register_operand" "r")
-			 (match_operand:SI 2 "register_operand" "r")])
-		   (match_operator:SI 5 "logical_shift_operator"
-			[(match_operand:SI 3 "register_operand" "r")
-			 (neg:SI (match_dup 2))])))]
+			 (match_operand:SI 3 "register_operand" "r")])
+		 (match_operator:SI 5 "logical_shift_operator"
+			[(match_operand:SI 2 "register_operand" "r")
+			 (neg:SI (match_dup 3))])]))]
   "!optimize_debug && optimize
    && xtensa_shlrd_which_direction (operands[4], operands[5]) != UNKNOWN"
 {
   switch (xtensa_shlrd_which_direction (operands[4], operands[5]))
     {
-    case ASHIFT:	return "ssl\t%2\;src\t%0, %1, %3";
-    case LSHIFTRT:	return "ssr\t%2\;src\t%0, %3, %1";
+    case ASHIFT:	return "ssl\t%3\;src\t%0, %1, %2";
+    case LSHIFTRT:	return "ssr\t%3\;src\t%0, %2, %1";
     default:		gcc_unreachable ();
     }
 }
@@ -1704,14 +1702,42 @@ 
    (set_attr "mode"	"SI")
    (set_attr "length"	"6")])
 
-(define_insn "*shlrd_const_<code>"
+(define_insn_and_split "*shlrd_reg"
+  [(set (match_operand:SI 0 "register_operand" "=a")
+	(match_operator:SI 6 "xtensa_bit_join_operator"
+		[(match_operator:SI 4 "logical_shift_operator"
+			[(match_operand:SI 1 "register_operand" "r")
+			 (neg:SI (match_operand:SI 3 "register_operand" "r"))])
+		 (match_operator:SI 5 "logical_shift_operator"
+			[(match_operand:SI 2 "register_operand" "r")
+			 (match_dup 3)])]))]
+  "!optimize_debug && optimize
+   && xtensa_shlrd_which_direction (operands[5], operands[4]) != UNKNOWN"
+  "#"
+  "&& 1"
+  [(set (match_dup 0)
+	(match_op_dup 6
+		[(match_op_dup 5
+			[(match_dup 2)
+			 (match_dup 3)])
+		 (match_op_dup 4
+			[(match_dup 1)
+			 (neg:SI (match_dup 3))])]))]
+  ""
+  [(set_attr "type"	"arith")
+   (set_attr "mode"	"SI")
+   (set_attr "length"	"6")])
+
+
+(define_insn "*shlrd_const"
   [(set (match_operand:SI 0 "register_operand" "=a")
-	(ior_op:SI (match_operator:SI 5 "logical_shift_operator"
+	(match_operator:SI 7 "xtensa_bit_join_operator"
+		[(match_operator:SI 5 "logical_shift_operator"
 			[(match_operand:SI 1 "register_operand" "r")
 			 (match_operand:SI 3 "const_int_operand" "i")])
-		   (match_operator:SI 6 "logical_shift_operator"
+		 (match_operator:SI 6 "logical_shift_operator"
 			[(match_operand:SI 2 "register_operand" "r")
-			 (match_operand:SI 4 "const_int_operand" "i")])))]
+			 (match_operand:SI 4 "const_int_operand" "i")])]))]
   "!optimize_debug && optimize
    && xtensa_shlrd_which_direction (operands[5], operands[6]) != UNKNOWN
    && IN_RANGE (INTVAL (operands[3]), 1, 31)
@@ -1729,16 +1755,17 @@ 
    (set_attr "mode"	"SI")
    (set_attr "length"	"6")])
 
-(define_insn "*shlrd_per_byte_<code>"
+(define_insn "*shlrd_per_byte"
   [(set (match_operand:SI 0 "register_operand" "=a")
-	(ior_op:SI (match_operator:SI 4 "logical_shift_operator"
+	(match_operator:SI 6 "xtensa_bit_join_operator"
+		[(match_operator:SI 4 "logical_shift_operator"
 			[(match_operand:SI 1 "register_operand" "r")
 			 (ashift:SI (match_operand:SI 2 "register_operand" "r")
 				    (const_int 3))])
-		   (match_operator:SI 5 "logical_shift_operator"
+		 (match_operator:SI 5 "logical_shift_operator"
 			[(match_operand:SI 3 "register_operand" "r")
 			 (neg:SI (ashift:SI (match_dup 2)
-					    (const_int 3)))])))]
+					    (const_int 3)))])]))]
   "!optimize_debug && optimize
    && xtensa_shlrd_which_direction (operands[4], operands[5]) != UNKNOWN"
 {
@@ -1753,32 +1780,34 @@ 
    (set_attr "mode"	"SI")
    (set_attr "length"	"6")])
 
-(define_insn_and_split "*shlrd_per_byte_<code>_omit_AND"
+(define_insn_and_split "*shlrd_per_byte_omit_AND"
   [(set (match_operand:SI 0 "register_operand" "=a")
-	(ior_op:SI (match_operator:SI 5 "logical_shift_operator"
+	(match_operator:SI 7 "xtensa_bit_join_operator"
+		[(match_operator:SI 5 "logical_shift_operator"
 			[(match_operand:SI 1 "register_operand" "r")
 			 (and:SI (ashift:SI (match_operand:SI 2 "register_operand" "r")
 					    (const_int 3))
 				 (match_operand:SI 4 "const_int_operand" "i"))])
-		   (match_operator:SI 6 "logical_shift_operator"
+		 (match_operator:SI 6 "logical_shift_operator"
 			[(match_operand:SI 3 "register_operand" "r")
 			 (neg:SI (and:SI (ashift:SI (match_dup 2)
 						    (const_int 3))
-					 (match_dup 4)))])))]
+					 (match_dup 4)))])]))]
   "!optimize_debug && optimize
    && xtensa_shlrd_which_direction (operands[5], operands[6]) != UNKNOWN
    && (INTVAL (operands[4]) & 0x1f) == 3 << 3"
   "#"
   "&& 1"
   [(set (match_dup 0)
-	(ior_op:SI (match_op_dup 5
+	(match_op_dup 7
+		[(match_op_dup 5
 			[(match_dup 1)
 			 (ashift:SI (match_dup 2)
 				    (const_int 3))])
-		   (match_op_dup 6
+		 (match_op_dup 6
 			[(match_dup 3)
 			 (neg:SI (ashift:SI (match_dup 2)
-					    (const_int 3)))])))]
+					    (const_int 3)))])]))]
   ""
   [(set_attr "type"	"arith")
    (set_attr "mode"	"SI")