https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123884
Jeffrey A. Law <law at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2026-06-23
Ever confirmed|0 |1
Blocks| |120763
Status|UNCONFIRMED |NEW
--- Comment #2 from Jeffrey A. Law <law at gcc dot gnu.org> ---
The problem I see with the patch is it doesn't preserve semantics.
(ior/xor (sign_extend (ashift (const_int 1) (reg X)))
(reg Y))
For XOR, consider if X is any value other than 31. In that case the upper 33
bits of the result are direct copies of the bits from reg Y. If X is the value
31, then the upper 33 bits of Y are inverted in the result.
For IOR, if X is any value other than 31, then the upper 33 bits are also
direct copies of the bits from reg Y. If X is 31, then the upper 33 bits in
the result are all on.
Those semantics aren't preserved in your patch. But I think I see a good path
forward.
We can query for the number of sign bits in Y. If the number of sign bits in Y
is 33, then the xor/ior + sext.w sequence preserves semantics. So a pattern
like this seems like it might work.
Note it's a define_split rather than a define_insn_and_split. If we're just
generating 2 insns, then it's highly advisable to use a define_split. It's
also the case that if we were to use a define_insn_and_split we can't guarantee
that the result of num_sign_bit_copies doesn't change. A pass like combine
uses a refined num_sign_bit_copies implementation, but the refinement isn't
necessarily valid outside the context of the combine pass. So we could end up
in a scenario where the insn's condition passes during combine, but fails
later.
Using a define_split avoids those problems. Once we make the transformation,
we don't care of something makes it harder to track num_sign_bit_copies for the
relevant pseudo register. Using a define_split also avoids the problems with
define_insn_and_split spoiling the cost model within the combine pass.
So I'm thinking a patch like this (very lightly tested):
(define_split
[(set (match_operand:DI 0 "register_operand")
(any_or:DI
(sign_extend:DI
(ashift:SI
(const_int 1)
(match_operand:QI 1 "register_operand")))
(match_operand:DI 2 "register_operand")))]
"(TARGET_64BIT
&& TARGET_ZBS
&& num_sign_bit_copies (operands[2], DImode) == 33)"
[(set (match_dup 0) (any_or:DI (ashift:DI (const_int 1) (match_dup 1))
(match_dup 2)))
(set (match_dup 0)
(sign_extend:DI (subreg:SI (match_dup 0) 0)))])
I'm going to sleep on this tonight, but it's the best option I've seen in this
space since I started working on RISC-V in 2022 and this issue first came to my
attention :-)
I'm putting this back on the weekly tracker to discuss in tomorrow's patchwork
meeting.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120763
[Bug 120763] [meta-bug] Tracker for bugs to visit during weekly RISC-V meeting
