RISC-V: Add bext pattern for ZBS
Checks
Commit Message
When (a & (1 << bit_no)) is tested inside an IF we can use a bit extract.
gcc/ChangeLog:
* config/riscv/bitmanip.md
(bext<mode>): Rename one to avoid name clash.
(branch<X:mode>_bext): New split pattern.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zbs-bext-02.c: New test.
---
gcc/config/riscv/bitmanip.md | 24 +++++++++++++++++++-
gcc/testsuite/gcc.target/riscv/zbs-bext-02.c | 18 +++++++++++++++
2 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-bext-02.c
Comments
On 5/4/23 11:08, Raphael Moreira Zinsly wrote:
> When (a & (1 << bit_no)) is tested inside an IF we can use a bit extract.
>
> gcc/ChangeLog:
>
> * config/riscv/bitmanip.md
> (bext<mode>): Rename one to avoid name clash.
> (branch<X:mode>_bext): New split pattern.
>
> gcc/testsuite/ChangeLog:
> * gcc.target/riscv/zbs-bext-02.c: New test.
> ---
> gcc/config/riscv/bitmanip.md | 24 +++++++++++++++++++-
> gcc/testsuite/gcc.target/riscv/zbs-bext-02.c | 18 +++++++++++++++
> 2 files changed, 41 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-bext-02.c
>
> diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> index a27fc3e34a1..e29e2d1fa53 100644
> --- a/gcc/config/riscv/bitmanip.md
> +++ b/gcc/config/riscv/bitmanip.md
> @@ -595,7 +595,7 @@
> ;; When performing `(a & (1UL << bitno)) ? 0 : -1` the combiner
> ;; usually has the `bitno` typed as X-mode (i.e. no further
> ;; zero-extension is performed around the bitno).
> -(define_insn "*bext<mode>"
> +(define_insn "*bext<mode>_2"
> [(set (match_operand:X 0 "register_operand" "=r")
> (zero_extract:X (match_operand:X 1 "register_operand" "r")
> (const_int 1)
This doesn't make sense to me. Is it possible this was from an earlier
version of the patch?
In general when we have a * prefix, we're allowed to have multiple
patterns with the same name. Essentially the pattern names are just for
debugging purposes, no API is exposed to generate those patterns when
there's a '*' prefix.
> @@ -720,6 +720,28 @@
> operands[9] = GEN_INT (clearbit);
> })
>
> +;; IF_THEN_ELSE: test for (a & (1 << BIT_NO))
> +(define_insn_and_split "*branch<X:mode>_bext"
> + [(set (pc)
> + (if_then_else
> + (match_operator 1 "equality_operator"
> + [(zero_extract:X (match_operand:X 2 "register_operand" "r")
> + (const_int 1)
> + (zero_extend:X (match_operand:QI 3 "register_operand" "r")))
> + (const_int 0)])
> + (label_ref (match_operand 0 "" ""))
> + (pc)))
> + (clobber (match_scratch:X 4 "=&r"))]
Formatting nit. In general the operands of a rtx operator all line up
together when we can. So in this case the (const_int 1) should line up
under the (match_operand:X 2). Similarly for the (zero_extend:X). That
may require wrapping the zero_extned line. The way to do that would be
to bring its match_operand down to a new line, indent it two spaces from
the open paren of the (zero_extend.
It's been a while since we poked at this, so maybe you've already told
me before, but would it make sense to use the GPR iterator rather than
the X iterator?
GPR would result in two patterns that are available to match at the same
time, one for SI, another for DI.
X also results in two patterns, but only one is available at any given
time dependent on TARGET_64BIT.
I guess the rest are defined in terms of X, particularly the bext<mode>
pattern. So nevermind, keep it as X.
So I think the only things we potentially adjust is to remove the hunk
which changes the name of the *bext<mode> pattern and the whitespace
fix. I think we'll be good to go after those changes.
Jeff
@@ -595,7 +595,7 @@
;; When performing `(a & (1UL << bitno)) ? 0 : -1` the combiner
;; usually has the `bitno` typed as X-mode (i.e. no further
;; zero-extension is performed around the bitno).
-(define_insn "*bext<mode>"
+(define_insn "*bext<mode>_2"
[(set (match_operand:X 0 "register_operand" "=r")
(zero_extract:X (match_operand:X 1 "register_operand" "r")
(const_int 1)
@@ -720,6 +720,28 @@
operands[9] = GEN_INT (clearbit);
})
+;; IF_THEN_ELSE: test for (a & (1 << BIT_NO))
+(define_insn_and_split "*branch<X:mode>_bext"
+ [(set (pc)
+ (if_then_else
+ (match_operator 1 "equality_operator"
+ [(zero_extract:X (match_operand:X 2 "register_operand" "r")
+ (const_int 1)
+ (zero_extend:X (match_operand:QI 3 "register_operand" "r")))
+ (const_int 0)])
+ (label_ref (match_operand 0 "" ""))
+ (pc)))
+ (clobber (match_scratch:X 4 "=&r"))]
+ "TARGET_ZBS"
+ "#"
+ "&& reload_completed"
+ [(set (match_dup 4) (zero_extract:X (match_dup 2)
+ (const_int 1)
+ (zero_extend:X (match_dup 3))))
+ (set (pc) (if_then_else (match_op_dup 1 [(match_dup 4) (const_int 0)])
+ (label_ref (match_dup 0))
+ (pc)))])
+
;; ZBKC or ZBC extension
(define_insn "riscv_clmul_<mode>"
[(set (match_operand:X 0 "register_operand" "=r")
new file mode 100644
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zbs -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-funroll-loops" } } */
+
+int
+foo(const long long B, int a)
+{
+ long long b = 1;
+ for (int sq = 0; sq < 64; sq++)
+ if (B & (b << sq))
+ a++;
+
+ return a;
+}
+
+/* { dg-final { scan-assembler-times "bext\t" 1 } } */
+/* { dg-final { scan-assembler-not "bset" } } */
+/* { dg-final { scan-assembler-not "and" } } */