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
