On 5/17/23 10:02, Jivan Hakobyan via Gcc-patches wrote:
> Subject:
> [v2] RISC-V: Remove masking third operand of rotate instructions
> From:
> Jivan Hakobyan via Gcc-patches <gcc-patches@gcc.gnu.org>
> Date:
> 5/17/23, 10:02
> 
> 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 (*<optab><mode>3_mask): New pattern,
>          combined from ...
>          (*<optab>si3_mask, *<optab>di3_mask): Here.
>          (*<optab><mode>3_mask_1): New pattern, combined from ...
>          (*<optab>si3_mask_1, *<optab>di3_mask_1): Here.
>          * 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.
> 
> 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
Thanks for the updated patch.  A few comments.


> 
> 
> 
> 
> -- 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>")])
So I couldn't resist the temptation to look at the mode iterator 
improvements a bit after our call this morning.  As you note, the most 
obvious changes will regress the testsuite.  But it turns out there are 
things we can do to further simplify/combine patterns.

So the trick for the above pattern is to use GPR2 rather than X for the 
mode of the bitwise-and subexpression.  That allows the mode of that 
subexpression to vary independently of the mode of the output. 
Similarly for the other pattern that you're adding to bitmanip.md.

We can use GPR/GPR2 iterators in the riscv.md changes as well.  The 
primary benefit in doing so is we can eliminate another pair of patterns.

With those change we have simpler code that still passes all the new tests.

I've regression tested this V3 variant with no issues.  I'll commit it 
to the trunk under your name since the bulk of the work is yours.

Thanks,
jeff