On 5/28/25 4:23 AM, Jiawei wrote:
This patch adds a peephole2 optimization that combines a 'bclr' followed by
a 'binv' into a single 'bset' instruction when the Zbs extension is enabled.
The motivation for this patch is that PR116398 limits 2→2 RTL combinations,
which prevents certain simplifications in the combiner pass. As a result,
combining 'bclr' and 'binv' through standard RTL combination is not feasible
when Zbs is enabled. An example is the testcase
g++.target/riscv/redundant-bitmap-2.C[1] from Jeff Law's patch[2].
PR116398:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=4d7a634f6d41029811cdcbd5f7282b5b07890094
[1] https://godbolt.org/z/dhYoTMY1v
[2]
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=05daf617ea22e1d818295ed2d037456937e23530
gcc/ChangeLog:
* config/riscv/bitmanip.md (*bset<mode>_2): New pattern.
* config/riscv/peephole.md: Ditto.
Signed-off-by: Jiawei <jia...@iscas.ac.cn>
---
gcc/config/riscv/bitmanip.md | 9 +++++++++
gcc/config/riscv/peephole.md | 16 ++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 21426f49679..1bd66c4aa19 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -615,6 +615,15 @@
"bset\t%0,x0,%1"
[(set_attr "type" "bitmanip")])
+(define_insn "*bset<mode>_2"
+ [(set (match_operand:X 0 "register_operand" "=r")
+ (ior:X (match_operand:X 1 "register_operand" "r")
+ (ashift:X (const_int 1)
+ (match_operand:QI 2 "register_operand" "r"))))]
+ "TARGET_ZBS"
+ "bset\t%0,%1,%2"
+ [(set_attr "type" "bitmanip")])
Isn't this case handled by this pattern:
(define_insn "*<bit_optab><mode>"
[(set (match_operand:X 0 "register_operand" "=r")
(any_or:X (ashift:X (const_int 1)
(match_operand:QI 2 "register_operand" "r"))
(match_operand:X 1 "register_operand" "r")))]
"TARGET_ZBS"
"<bit_optab>\t%0,%1,%2"
[(set_attr "type" "bitmanip")])
Oh, it's the operand order. Your pattern isn't canonical. If something
generated that form, it needs to be fixed. Here's the relevant passage
that covers canonicalization of associative operators:
For associative operators, a sequence of operators will always chain
to the left; for instance, only the left operand of an integer @code{plus}
can itself be a @code{plus}. @code{and}, @code{ior}, @code{xor},
@code{plus}, @code{mult}, @code{smin}, @code{smax}, @code{umin}, and
@code{umax} are associative when applied to integers, and sometimes to
floating-point.
+
;; The result will always have bits 32..63 clear, so the zero-extend
;; is redundant. We could split it to bset<mode>_1, but it seems
;; unnecessary.
diff --git a/gcc/config/riscv/peephole.md b/gcc/config/riscv/peephole.md
index b5cc1924c76..1d5d15e9005 100644
--- a/gcc/config/riscv/peephole.md
+++ b/gcc/config/riscv/peephole.md
@@ -39,6 +39,22 @@
operands[5] = GEN_INT (INTVAL (operands[2]) - INTVAL (operands[5]));
})
+;; ZBS
+(define_peephole2
+ [(set (match_operand:X 1 "register_operand")
+ (and:X (rotate:X (const_int -2)
+ (match_operand:QI 3 "register_operand"))
+ (match_operand:X 2 "register_operand")))
+ (set (match_operand:X 0 "register_operand")
+ (xor:X (ashift:X (const_int 1)
+ (match_dup 3))
+ (match_dup 1)))]
+ "TARGET_ZBS"
+ [(set (match_dup 0)
+ (ior:X (match_dup 2)
+ (ashift:X (const_int 1)
+ (match_dup 3))))])
This seems like it would be much better as a combine pattern. In fact,
I'm a bit surprised that combine didn't simplify this series of
operations into a IOR. So I'd really like to see the .combine dump with
and without this hunk for the relevant testcase.
Also this patch should include a testcase or an addition/adjustment to
an existing testcase.
jeff
