[PATCH] RISC-V: Add bext pattern for ZBS

[PATCH] RISC-V: Add bext pattern for ZBS

[Raphael Moreira Zinsly]

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)
@@ -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")
diff --git a/gcc/testsuite/gcc.target/riscv/zbs-bext-02.c b/gcc/testsuite/gcc.target/riscv/zbs-bext-02.c
new file mode 100644
index 00000000000..3f3b8404eca
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zbs-bext-02.c
@@ -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" } } */
-- 
2.40.0

Re: [PATCH] RISC-V: Add bext pattern for ZBS

[Jeff Law]

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