* [v2] RISC-V: Remove masking third operand of rotate instructions
@ 2023-05-17 16:02 Jivan Hakobyan
2023-05-17 19:06 ` Jeff Law
0 siblings, 1 reply; 3+ messages in thread
From: Jivan Hakobyan @ 2023-05-17 16:02 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1.1: Type: text/plain, Size: 1829 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 (*<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
--
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: [v2] RISC-V: Remove masking third operand of rotate instructions
2023-05-17 16:02 [v2] RISC-V: Remove masking third operand of rotate instructions Jivan Hakobyan
@ 2023-05-17 19:06 ` Jeff Law
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Law @ 2023-05-17 19:06 UTC (permalink / raw)
To: Jivan Hakobyan, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4217 bytes --]
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
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 15683 bytes --]
commit 6da6ed95c9ca247d405da3dfb737b743686fe5e6
Author: Jivan Hakobyan <jivanhakobyan9@gmail.com>
Date: Wed May 17 13:00:28 2023 -0600
RISC-V: Remove masking third operand of rotate instructions
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><GPR:mode>3_mask): New pattern,
combined from ...
(*<optab>si3_mask, *<optab>di3_mask): Here.
(*<optab>si3_mask_1, *<optab>di3_mask_1): And here.
* config/riscv/bitmanip.md (*<bitmanip_optab><GPR:mode>3_mask): New
pattern.
(*<bitmanip_optab>si3_sext_mask): Likewise.
* config/riscv/iterators.md (shiftm1): Use const_si_mask_operand
and const_di_mask_operand.
(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:
* gcc.target/riscv/shift-and-2.c: Fixed test
* gcc.target/riscv/zbb-rol-ror-01.c: New test
* gcc.target/riscv/zbb-rol-ror-02.c: New test
* gcc.target/riscv/zbb-rol-ror-03.c: New test
* gcc.target/riscv/zbb-rol-ror-04.c: New test
* gcc.target/riscv/zbb-rol-ror-05.c: New test
* gcc.target/riscv/zbb-rol-ror-06.c: New test
* gcc.target/riscv/zbb-rol-ror-07.c: New test
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index a27fc3e34a1..6f3d24a8a88 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -351,6 +351,42 @@ (define_insn "rotlsi3_sext"
"rolw\t%0,%1,%2"
[(set_attr "type" "bitmanip")])
+(define_insn_and_split "*<bitmanip_optab><GPR:mode>3_mask"
+ [(set (match_operand:GPR 0 "register_operand" "= r")
+ (bitmanip_rotate:GPR
+ (match_operand:GPR 1 "register_operand" " r")
+ (match_operator 4 "subreg_lowpart_operator"
+ [(and:GPR2
+ (match_operand:GPR2 2 "register_operand" "r")
+ (match_operand 3 "<GPR:shiftm1>" "<GPR:shiftm1p>"))])))]
+ "TARGET_ZBB || TARGET_ZBKB"
+ "#"
+ "&& 1"
+ [(set (match_dup 0)
+ (bitmanip_rotate:GPR (match_dup 1)
+ (match_dup 2)))]
+ "operands[2] = gen_lowpart (QImode, operands[2]);"
+ [(set_attr "type" "bitmanip")
+ (set_attr "mode" "<GPR: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:GPR
+ (match_operand:GPR 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 UNITMODE [(HF "HF") (SF "SF") (DF "DF")])
(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 bitmanip_minmax [smin umin smax umax])
(define_code_iterator clz_ctz_pcnt [clz ctz popcount])
+(define_code_iterator bitmanip_rotate [rotate rotatert])
+
;; -------------------------------------------------------------------
;; Code Attributes
;; -------------------------------------------------------------------
@@ -271,7 +273,9 @@ (define_code_attr bitmanip_optab [(smin "smin")
(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 @@ (define_predicate "not_single_bit_mask_operand"
(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 a9179931217..e773bc748bf 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2048,45 +2048,6 @@ (define_insn "<optab>si3"
[(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")
- (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"
- "#"
- "&& 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 "<optab>di3"
[(set (match_operand:DI 0 "register_operand" "= r")
(any_shift:DI
@@ -2103,45 +2064,23 @@ (define_insn "<optab>di3"
[(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")
+(define_insn_and_split "*<optab><GPR: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: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")
- (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"
+ [(and:GPR2
+ (match_operand:GPR2 2 "register_operand" "r")
+ (match_operand 3 "<GPR:shiftm1>"))])))]
+ ""
"#"
"&& 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")
@@ -2164,34 +2103,10 @@ (define_insn_and_split "*<optab>si3_extend_mask"
(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: [v2] RISC-V: Remove masking third operand of rotate instructions
2023-05-18 15:37 Joern Rennecke
@ 2023-05-18 16:08 ` Joern Rennecke
0 siblings, 0 replies; 3+ messages in thread
From: Joern Rennecke @ 2023-05-18 16:08 UTC (permalink / raw)
To: GCC Patches, Jivan Hakobyan; +Cc: Jeff Law
[-- Attachment #1: Type: text/plain, Size: 580 bytes --]
On Thu, 18 May 2023 at 16:37, Joern Rennecke <joern.rennecke@embecosm.com> wrote
in https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618928.html :
>
> This breaks building libstdc++-v3 for
> -march=rv32imafdcv_zicsr_zifencei_zba_zbb_zbc_zbs_zve32f_zve32x_zve64d_zve64f_zve64x_zvl128b_zvl32b_zvl64b
> -mabi=ilp32f .
Sorry, I forgot the ChangeLog entry for my patch and missed the [v2]
part of the subject.
2023-05-18 Joern Rennecke <joern.rennecke@embecosm.com>
gcc/ChangeLog:
* config/riscv/constraints.md (DsS, DsD): Restore agreement
with shiftm1 mode attribute.
[-- 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"
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-18 16:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-17 16:02 [v2] RISC-V: Remove masking third operand of rotate instructions Jivan Hakobyan
2023-05-17 19:06 ` Jeff Law
2023-05-18 15:37 Joern Rennecke
2023-05-18 16:08 ` [v2] " 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).