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

[Max Filippov <jcmvbkbc@gmail.com>]

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.

-- 
Thanks.
-- Max