On 10/21/26 7:00 AM, Milan Tripkovic wrote:
This patch contains 1 split and 1 insn_and_split pattern:
The define_split handles 3rd case (shafinish). On RV64
~(1 << n) for an int n is computed in SImode and sign-extended to
DImode. If the other operand has no bits set above bit 30,
the sign extension does not matter, and the AND
can be rewritten with (rotate (const_int -2) n), which matches bclr.
void shafinish(unsigned char *block, int blockcnt)
{
block[blockcnt] |= (0x01 << (blockcnt));
block[blockcnt] &= ~(0x01 << (blockcnt));
}
before:
add a0,a0,a1
lbu a4,0(a0)
li a5,1
sllw a5,a5,a1
andn a5,a4,a5
sb a5,0(a0)
after:
add a0,a0,a1
lbu a5,0(a0)
bclr t0,a5,a1
sb t0,0(a0)
The define_insn_and_split removes the redundant andi in the 1st
example (shabits). The shift count is (~x) & C, which
is at most C, so when C < log2 (M + 1) the bit always lies inside the
mask M and the AND is a no-op. The pattern splits into li/andn/bset.
There should be a 2nd patch that covers the GIMPLE side of the problem
for the 1st and 2nd example.
Regression tested on riscv64.
2026-10-21 Milan Tripkovic <[email protected]>
PR target/124020
gcc/ChangeLog:
*config/riscv/bitmanip.md (bset_andn_masked): new pattern
gcc/testsuite/ChangeLog:
*gcc.target/riscv/pr124020-1.c: New test.
*gcc.target/riscv/pr124020-2.c: New test.
So this is great. I've been slowly cobbling together a prompt for
reviewing .md files and changes and I can test it against your patch!
That reviewer flagged shift/rotate count issues in your define_split.
The input/outputs are SI/DI respectively which can lead to a mis-compile
because the hardware handles the shift count differently for SI and DI
shifts. The former are masked with 0x1f by the hardware, the latter by
0x3f.
A shift count like 0x20 could slip through here. It's a fairly obscure
sequence of events, but it can happen (I know because I made a similar
mistake a few years ago, then had to debug the failure...) Essentially
combine+target can eliminate masking that appears in the source knowing
that the hardware with mask the shift count with 0x1f for an SI shift.
So we can get scenarios where shift count register could have a value
like 0x21. When you turn that into DI for the output pattern, the shift
count of 0x21 is valid and the hardware acts accordingly.
So if a value like 0x21 slipped through, the original RTL would clear
bit #1. The split RTL would clear bit #33. Opps.
One idea would be to preserve the SI shift so generating something like
(set (match_dup 0)
(and:DI (sign_extend:DI (rotate:SI (const_int -2) (match_dup 1))
(match_dup 2)))
Or something like that. But I suspect that's not going to match any
known define_insn.
The other approach would be to generate two insns in your split rtl.
The first would mask the shift count against 0x1f, the second would be
the bitclear using the masked shift count.
That means its a 3->2 split, which should be OK. In fact, I'm not 100%
sure a 3->1 split works. Generally 3->1 we'd want to create a
define_insn. But that won't work here because you can't use
nonzero_bits in the condition of a define_insn. A 3->2 split would
still be an improvement.
I haven't walked through it completely, but we generally try to avoid
define_insn_and_split. I think we can turn that into a define_split as
well.
Drop the first (set (match_dup 4) (match_dup 2) in the output RTL and
change the (match_dup 4) operand to the AND to (match_dup 2).
Why was the if (GET_CODE (operands[4]) == SCRATCH) clause needed? I
would have expected this to split only after reload in which case the
match_scratch would have been replaced with a temporary register. Note
that if we change to a define_split, then we generally use (clobber
(match_operand:DI 4 "register_operand") instead of the scratch expression.
Anyway, the approach you're taking by restricting the other operand of
the AND is interesting and I think holds promise for using the Zbs
extension in ways we're currently missing. We just have to be really
careful on the semantics. So probably need a V2 here.
jeff
CONFIDENTIALITY: The contents of this e-mail are confidential and
intended only for the above addressee(s). If you are not the intended
recipient, or the person responsible for delivering it to the intended
recipient, copying or delivering it to anyone else or using it in any
unauthorized manner is prohibited and may be unlawful. If you receive
this e-mail by mistake, please notify the sender and the systems
administrator at [email protected] immediately.
