On Tue, Jul 7, 2026 at 10:38 AM Dusan Stojkovic
<[email protected]> wrote:
>
> This commit expands on the work by Jeff and Milan regarding generating
> more Zbs instructions for RISC-V (PR123884).
>
> For the testcase:
> void foo(unsigned char *data, unsigned int lo_bit) {
> unsigned int mask = ((1UL << 1) - 1) << lo_bit;
> *data = (*data & ~mask) | ((1 << lo_bit) & mask)
> }
>
> Without their patch the following assembly would be produced:
> foo(unsigned char*, unsigned int):
> lbu a4,0(a0)
> li a5,1
> sllw a5,a5,a1
> xor a5,a4,a5
> bset a1,x0,a1
> and a5,a5,a1
> xor a4,a4,a5
> sb a4,0(a0)
> ret
>
> With this patch it would be transformed into:
> foo(unsigned char*, unsigned int):
> lbu a5,0(a0)
> li a4,1
> sllw a4,a4,a1
> bclr a5,a5,a1
> or a5,a5,a4
> sb a5,0(a0)
> ret
>
> Thus, in the combine pass arises a new pattern to try:
> Trying 17, 18 -> 22:
> 17: r155:SI=0x1
> 18: r156:DI=sign_extend(r155:SI<<r143:DI#0)
> REG_DEAD r155:SI
> REG_DEAD r143:DI
> REG_EQUAL sign_extend(0x1<<r143:DI#0)
> 22: r159:DI=r152:DI|r156:DI
> REG_DEAD r156:DI
> REG_DEAD r152:DI
> Failed to match this instruction:
> (set (reg:DI 159)
> (ior:DI (sign_extend:DI (ashift:SI (const_int 1 [0x1])
> (subreg:QI (reg/v:DI 143 [ lo_bitD.2469 ]) 0)))
> (reg:DI 152)))
> Splitting with gen_split_161 (bitmanip.md:796) <- pattern added in PR123884
> Successfully matched this instruction:
> (set (reg:DI 159)
> (ior:DI (ashift:DI (const_int 1 [0x1])
> (subreg:QI (reg/v:DI 143 [ lo_bitD.2469 ]) 0))
> (reg:DI 152)))
> Successfully matched this instruction:
> (set (reg:DI 159)
> (sign_extend:DI (subreg:SI (reg:DI 159) 0)))
>
> And ultimately generates the following assembly:
> foo:
> lbu a5,0(a0)
> bset a5,a5,a1
> sb a5,0(a0)
> ret
>
> The match.pd pattern is inspired by Jeff's notes in the bugreport.
>
> Not all testcases added by this patch simplify completely.
> *For rv32 the smaller type to ULL produces poor assembly still
> *For rv64 the UI to UL/ULL are still not clean.
>
> Regression tested on x86, rv64 and rv32.
>
> 2026-07-07 Dusan Stojkovic <[email protected]>
>
> PR target/123883
>
> gcc/ChangeLog:
>
> * match.pd: New pattern.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/pr123883.c: New test.
>
> Co-authored-by: Jeff Law <[email protected]>
Some fixes:
+/* (T)(1 << x) & (T)(1 << x) -> (T)(1 << x),
+ * but keep the shift in the wider type to avoid introducing
+ * undefined behaviour. */
+(simplify
+ (bit_and:c
+ (convert? (lshift@2 integer_onep@1 @0))
+ (convert? (lshift@3 integer_onep@4 @0)))
+ (with
+ {
+ tree ltype0 = TREE_TYPE (@2);
+ tree ltype1 = TREE_TYPE (@3);
+ tree largertype = ltype0;
+ tree largertypeone = build_one_cst (largertype);
Remove largertypeone.
+ }
+ (if (INTEGRAL_TYPE_P (type)
+ && INTEGRAL_TYPE_P (ltype0)
+ && INTEGRAL_TYPE_P (ltype1)
+ && element_precision (type) <= element_precision (ltype0)
+ && element_precision (type) <= element_precision (ltype1))
So the check should be:
&& element_precision (type) <= element_precision (ltype1)
&& element_precision (ltype1) <= element_precision (ltype0))
Such that `type <= itype1 <= itype0`
+ (convert:type
+ (lshift:largertype { largertypeone; } @0)))))
Replace `{ largertypeone; }` with `@1`.
Even though INTEGER_CSTs are shared and small integers are kept
around; this makes the code slightly faster as you don't need to look
up the constant.
if you want a future expansion on this, a nice change would remove the
`:c` and add a check for the other way around; that is `type <=
itype0 <= itype1` and using itype1 for the inner type.
Thanks,
Andrea
>
>
>
>
> 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.
