https://gcc.gnu.org/g:576fead3848b9288a914d9d10333de36ec791a57
commit r17-1855-g576fead3848b9288a914d9d10333de36ec791a57 Author: Milan Tripkovic <[email protected]> Date: Thu Jun 25 07:34:14 2026 -0600 [RISC-V][PR target/123884] Generate more Zbs instructions for RISC-V So between Milan's work and my own realization earlier this week, we can finally get resolution on pr123884 and pr106585. The core issue is for what appear to be natural looking single bit manipulations we often struggle to generate the bset, bclr or binv on riscv64. A few items get in the way. First, rv64 (of course) promotes sub-word objects (ie ints) into word sized objects. Second, we do have *some* instructions that work on sub-objects (such as shifts which sign extend out to 64 bits). Third, a 32-bit object on rv64 is supposed to be sign extended out to 64 bits when it "escapes" or gets used in a comparison. Finally, the Zbs instructions do not have forms which sign extend out to 64 bits if the SI sign bit is changed. Consider this reduced fragment from x264: int x264_macroblock_encode_p8x8(int dst, int a) { dst &= ~(1 << a); return dst; } The code going into ext-dce looks like this: > (insn 7 4 8 2 (set (reg:SI 141) > (const_int 1 [0x1])) "j.c":3:16 276 {*movsi_internal} > (nil)) > (insn 8 7 10 2 (set (reg:DI 142) > (sign_extend:DI (ashift:SI (reg:SI 141) > (subreg:QI (reg/v:DI 138 [ a ]) 0)))) "j.c":3:16 312 {ashlsi3_extend} > (expr_list:REG_DEAD (reg:SI 141) > (expr_list:REG_DEAD (reg/v:DI 138 [ a ]) > (expr_list:REG_EQUAL (sign_extend:DI (ashift:SI (const_int 1 [0x1]) > (subreg:QI (reg/v:DI 138 [ a ]) 0))) > (nil))))) > (insn 10 8 12 2 (set (reg:DI 143) > (not:DI (reg:DI 142))) "j.c":3:12 113 {one_cmpldi2} > (expr_list:REG_DEAD (reg:DI 142) > (nil))) > (insn 12 10 13 2 (set (reg:DI 145) > (and:DI (reg/v:DI 137 [ dst ]) > (reg:DI 143))) "j.c":3:9 104 {*anddi3} > (expr_list:REG_DEAD (reg:DI 143) > (expr_list:REG_DEAD (reg/v:DI 137 [ dst ]) > (nil)))) > (insn 13 12 18 2 (set (reg:DI 146) > (sign_extend:DI (subreg:SI (reg:DI 145) 0))) "j.c":4:12 discrim 1 125 {*extendsidi2_internal} > (expr_list:REG_DEAD (reg:DI 145) > (nil))) That'll end up generating something like: > li a5,1 # 7 [c=4 l=4] *movsi_internal/1 > sllw a1,a5,a1 # 8 [c=8 l=4] ashlsi3_extend > not a1,a1 # 10 [c=4 l=4] one_cmpldi2 > and a0,a0,a1 # 18 [c=4 l=4] *anddi3/0 That seems almost tailor made for bclr. Except for the possibility that we're clearing bit 31. THe RTL above would clear bits 31..63. bclr just clears one bit. ext-dce comes along and realizes that insn 13 is redundant and removes it. But we still have the problem that the RTL would clear bits 31..63 if shift count in (reg:DI 138) is 31 and thus trying to combine the result and generate a bclr is wrong. We had the idea that we could follow the bclr with a sign extension. That works for this testcase, but is wrong in general. The insight from earlier this week is the semantics of bclr+sext work when (reg:DI 137) has 33 or more sign bit copies. In that limited, but common, case the semantics of bclr+sext match the RTL exactly. Just as important the output is 2 insns, so we can model it as a simple define_split rather than a define_insn_and_split. So I've adjusted Milan's patch to check the number of sign bit copies in the object where we want to clear a single bit. If the number of sign bit copies is 33 or more, then we allow the splitter to trigger. The net is we get: > bclr a0,a0,a1 # 10 [c=4 l=4] *bclrdi > sext.w a0,a0 # 18 [c=4 l=4] *extendsidi2_internal/0 The same idea works for bset and binv. This has been bootstrapped and regression tested on both the c920 (where it should do nothing, no Zbs extension) and the K3 (where I turn on Zbs by default). It's also been regression tested on riscv32-elf and riscv64-elf. I'll obviously wait for pre-commit CI to run before moving forward. But I think we've finally got this issue nailed down. PR target/123884 PR target/106585 gcc/ * config/riscv/bitmanip.md: Add new splits for single bit manipulation cases followed by a sign extension. gcc/testsuite * gcc.target/riscv/pr123884-a.c: New test. * gcc.target/riscv/pr123884-b.c: New test. * gcc.target/riscv/pr123884-c.c: New test. Co-authored-by: Jeff Law <[email protected]> Diff: --- gcc/config/riscv/bitmanip.md | 43 +++++++++++++++++++++++++++++ gcc/testsuite/gcc.target/riscv/pr123884-a.c | 13 +++++++++ gcc/testsuite/gcc.target/riscv/pr123884-b.c | 14 ++++++++++ gcc/testsuite/gcc.target/riscv/pr123884-c.c | 13 +++++++++ 4 files changed, 83 insertions(+) diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md index bd2aaf0587eb..fd0f85d10f39 100644 --- a/gcc/config/riscv/bitmanip.md +++ b/gcc/config/riscv/bitmanip.md @@ -783,6 +783,49 @@ { operands[2] = GEN_INT (INTVAL (operands[2]) & 0x1f); } [(set_attr "type" "bitmanip")]) +;; The goal here is to utilize the Zbs extension on SImode values when doing +;; so is safe. The problem is if we change the SI sign bit, the sign bit +;; does not propagate to bits 32..63. That makes preserving semantics of +;; this kind of RTL harder and more generally makes using the Zbs extension +;; harder. +;; +;; However, if we know the *other* operand has at least 33 sign bit copies, +;; then we can use the Zbs intruction followed by an sign extension. So the +;; result is a 3->2 split and we'll still have a chance to eliminate the +;; trailing sign extension. +(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)))]) + +(define_split + [(set (match_operand:DI 0 "register_operand") + (and:DI (not: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) (and:DI (rotate:DI (const_int -2) (match_dup 1)) + (match_dup 2))) + (set (match_dup 0) + (sign_extend:DI (subreg:SI (match_dup 0) 0)))]) + + ;; Similarly two patterns for AND generating bclr to ;; manipulate a bit in a register (define_insn_and_split "" diff --git a/gcc/testsuite/gcc.target/riscv/pr123884-a.c b/gcc/testsuite/gcc.target/riscv/pr123884-a.c new file mode 100644 index 000000000000..fb36da04052e --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr123884-a.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zbs -mabi=lp64d" { target rv64 } } */ +/* { dg-options "-march=rv32gc_zbs -mabi=ilp32" { target rv32 } } */ +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og"} } */ + +int foo(int dst, int a) +{ + dst ^= (1 << a); + return dst; +} + +/* { dg-final { scan-assembler-times "binv" 1 } } */ +/* { dg-final { scan-assembler-times "sext.w" 1 { target rv64 } } } */ diff --git a/gcc/testsuite/gcc.target/riscv/pr123884-b.c b/gcc/testsuite/gcc.target/riscv/pr123884-b.c new file mode 100644 index 000000000000..8ea33cbb32dd --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr123884-b.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zbs -mabi=lp64d" { target rv64 } } */ +/* { dg-options "-march=rv32gc_zbs -mabi=ilp32" { target rv32 } } */ +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */ + +int x264_macroblock_encode_p8x8(int dst, int a) +{ + dst &= ~(1 << a); + return dst; +} + +/* { dg-final { scan-assembler-times "bclr" 1 } } */ +/* { dg-final { scan-assembler-times "sext.w" 1 { target rv64 } } } */ + diff --git a/gcc/testsuite/gcc.target/riscv/pr123884-c.c b/gcc/testsuite/gcc.target/riscv/pr123884-c.c new file mode 100644 index 000000000000..10f42a305457 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr123884-c.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zbs -mabi=lp64d" { target rv64 } } */ +/* { dg-options "-march=rv32gc_zbs -mabi=ilp32" { target rv32 } } */ +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og"} } */ + +int foo2(int dst, int a) +{ + dst |= (1 << a); + return dst; +} + +/* { dg-final { scan-assembler-times "bset" 1 } } */ +/* { dg-final { scan-assembler-times "sext.w" 1 { target rv64} } } */
