[RISC-V][PR target/64345][PR tree-optimization/80770] Improve simple bit extractions...

Whee.  So what got me wandering down this path was looking for a good bug for
Shreya or Austin and concluding this one would be dreadful for both 🙂

We're basically looking at single bit extractions where there's a bit-not
somewhere in the sequence.

A few examples for the motivating PR64345.  They were for the SH, but aren't
handled well for RISC-V either.

> unsigned int test0 (unsigned int x)
> {
>   return ((x >> 4) ^ 1) & 1;
> }
>
> unsigned int test1 (unsigned int x)
> {
>   return ((x >> 4) & 1) ^ 1;
> }
>
> unsigned int test2 (unsigned int x)
> {
>   return ~(x >> 4) & 1;
> }

Right now those generates sequences like this:

>         li      a5,1
>         srliw   a0,a0,4
>         andn    a0,a5,a0

But we can do better.  This is semantically equivalent, two bytes shorter and
at least as fast.

>         xori    a0,a0,16        # 8     [c=4 l=4]  *xordi3/1
>         bexti   a0,a0,4 # 16    [c=4 l=4]  *bexti

The core problem is the little white lie we have for and-not:

> (define_insn_and_split "*<optab>_not_const<mode>"
>   [(set (match_operand:X 0 "register_operand" "=r")
>        (bitmanip_bitwise:X (not:X (match_operand:X 1 "register_operand" "r"))
>               (match_operand:X 2 "const_arith_operand" "I")))

There is no such insn.  andn, orn, xorn do not accept constants. But pretending
we do may help generate better performing code in some cases.

That pattern is a single insn from combine's standpoint.  So when we see this:

> Trying 6 -> 10:
>     6: r140:DI=zero_extract(r145:DI,0x1c,0x4)
>       REG_DEAD r145:DI
>    10: {r143:DI=~r140:DI&0x1;clobber scratch;}
>       REG_DEAD r140:DI
> Failed to match this instruction:
> (parallel [
>         (set (reg:DI 143)
>             (zero_extract:DI (xor:DI (reg:DI 145 [ x ])
>                     (const_int 16 [0x10]))
>                 (const_int 1 [0x1])
>                 (const_int 4 [0x4])))
>         (clobber (scratch:DI))
>     ])

We can't split it because the result would be 2 insns and it was already 2
insns from combine's standpoint (the little while lie shows up in insn 10 which
is really 2 instructions, but just one insn).

I looked at the wacky possibility of making the problem pattern only available
after reload in the hopes that late-combine could generate it, but late-combine
doesn't handle scratches/clobbers like that.

I consider the cases where the lie helps code generation very much on the
margins and realized that we could turn it into a peephole2.  That way we don't
regress on those marginal cases, but the problem pattern doesn't get in the way
of combine's work.

So that's a good first step. But not entirely sufficient to get the best
possible code for those tests.  In particular, given equal costs this patch
also steers towards AND which has the advantage that on an OoO core the
constant load in that case can sometimes issue for free or it might be
encodable directly as well.  On an in-order core it gives the scheduler more
freedom.

There's a bit of tension on that topic and some issues I'm not trying to tackle
at this time.  Essentially in the sign-bit-splat path, depending on the
constants different paths might be preferred (particularly when there's a NOT
in the sequence).  It's on the margins and touched on by a different BZ.

The net is we can fix the various extraction problems on RISC-V exposed by the
testcases in PR64345 without regressing the minor and-not cases the
define_insn_and_split was handling with one less define_insn_and_split in the
port.  It likely improves pr80770 as well, though I haven't checked that.

Bootstrapped and regression tested on both the Pioneer and BPI.  Also
regression tested on riscv64-elf and riscv32-elf.  And since this touched
ifcvt.cc, bootstrapped and regression tested on x86_64 as well 🙂  It's also
regression tested across all the embedded targets in my tester.

	PR target/64345
	PR tree-optimization/80770
gcc/

	* config/riscv/bitmanip.md (<optab>_not_const<mode>): Turn into a
	peephole2 to avoid matching prior to combine.
	* ifcvt.cc (noce_try_sign_bit_splat): When costs are equal steer
	towards an AND based sequence.

gcc/testsuite/
	* gcc.target/riscv/pr120553-2.c: Update expected output.
	* gcc.target/riscv/pr64345.c: New test.
	* gcc.target/riscv/zbb-andn-orn-01.c: Skip when peephole2 isn't run.
	* gcc.target/riscv/zbb-andn-orn-02.c: Likewise.

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 59b71ed263b0aabb11aea468cdec0fbf88d043aa..697198fcc913a4eec5ef9a580a63612fbd437dc2 100644 (file)
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -1,4 +1,4 @@
-;); Machine description for RISC-V Bit Manipulation operations.
+;; Machine description for RISC-V Bit Manipulation operations.
 ;; Copyright (C) 2021-2025 Free Software Foundation, Inc.
 
 ;; This file is part of GCC.
@@ -237,19 +237,20 @@
   [(set_attr "type" "bitmanip")
    (set_attr "mode" "<X:MODE>")])
 
-(define_insn_and_split "*<optab>_not_const<mode>"
-  [(set (match_operand:X 0 "register_operand" "=r")
-       (bitmanip_bitwise:X (not:X (match_operand:X 1 "register_operand" "r"))
-              (match_operand:X 2 "const_arith_operand" "I")))
-  (clobber (match_scratch:X 3 "=&r"))]
+(define_peephole2
+  [(match_scratch:X 4 "r")
+   (set (match_operand:X 0 "register_operand")
+       (not:X (match_operand:X 1 "register_operand")))
+   (set (match_operand:X 2 "register_operand")
+       (bitmanip_bitwise:X (match_dup 0)
+                           (match_operand 3 "const_int_operand")))
+   (match_dup 4)]
   "(TARGET_ZBB || TARGET_ZBKB) && !TARGET_ZCB
-   && !optimize_function_for_size_p (cfun)"
-  "#"
-  "&& reload_completed"
-  [(set (match_dup 3) (match_dup 2))
-   (set (match_dup 0) (bitmanip_bitwise:X (not:X (match_dup 1)) (match_dup 3)))]
-  ""
-  [(set_attr "type" "bitmanip")])
+   && !optimize_function_for_size_p (cfun)
+   && rtx_equal_p (operands[0], operands[2])
+   && riscv_const_insns (operands[3], false) == 1"
+  [(set (match_dup 4) (match_dup 3))
+   (set (match_dup 0) (bitmanip_bitwise:X (not:X (match_dup 1)) (match_dup 4)))])
 
 ;; '(a >= 0) ? b : 0' is emitted branchless (from if-conversion).  Without a
 ;; bit of extra help for combine (i.e., the below split), we end up emitting

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index 3dcb1be486923c33081fb22bf1fe6d6002aeafed..97ef09a7a68064e512e4060ccc316cb993f88a31 100644 (file)
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -1289,13 +1289,13 @@ noce_try_sign_bit_splat (struct noce_if_info *if_info)
       bool speed_p = optimize_insn_for_speed_p ();
       if (exact_log2 (val_a + 1) >= 0
          && (rtx_cost (shift_right, mode, SET, 1, speed_p)
-             <= rtx_cost (and_form, mode, SET, 1, speed_p)))
+             < rtx_cost (and_form, mode, SET, 1, speed_p)))
        temp = expand_simple_binop (mode, LSHIFTRT, temp,
                                    GEN_INT (rshift_count),
                                    if_info->x, false, OPTAB_WIDEN);
       else if (exact_log2 (~val_a + 1) >= 0
               && (rtx_cost (shift_left, mode, SET, 1, speed_p)
-                  <= rtx_cost (and_form, mode, SET, 1, speed_p)))
+                  < rtx_cost (and_form, mode, SET, 1, speed_p)))
        temp = expand_simple_binop (mode, ASHIFT, temp,
                                    GEN_INT (ctz_hwi (val_a)),
                                    if_info->x, false, OPTAB_WIDEN);
@@ -1341,13 +1341,13 @@ noce_try_sign_bit_splat (struct noce_if_info *if_info)
       bool speed_p = optimize_insn_for_speed_p ();
       if (exact_log2 (val_b + 1) >= 0
          && (rtx_cost (shift_right, mode, SET, 1, speed_p)
-             <= rtx_cost (and_form, mode, SET, 1, speed_p)))
+             < rtx_cost (and_form, mode, SET, 1, speed_p)))
        temp = expand_simple_binop (mode, LSHIFTRT, temp,
                                    GEN_INT (rshift_count),
                                    if_info->x, false, OPTAB_WIDEN);
       else if (exact_log2 (~val_b + 1) >= 0
               && (rtx_cost (shift_left, mode, SET, 1, speed_p)
-                  <= rtx_cost (and_form, mode, SET, 1, speed_p)))
+                  < rtx_cost (and_form, mode, SET, 1, speed_p)))
        temp = expand_simple_binop (mode, ASHIFT, temp,
                                    GEN_INT (ctz_hwi (val_b)),
                                    if_info->x, false, OPTAB_WIDEN);

diff --git a/gcc/testsuite/gcc.target/riscv/pr120553-2.c b/gcc/testsuite/gcc.target/riscv/pr120553-2.c
index 1501f8654d9a8509372d3b78e1fd60a093735b57..000f4bb687cdbdbf00b9b62b7be910347a448db3 100644 (file)
--- a/gcc/testsuite/gcc.target/riscv/pr120553-2.c
+++ b/gcc/testsuite/gcc.target/riscv/pr120553-2.c
@@ -83,8 +83,8 @@ T1(63)
 #endif
 
 /* { dg-final { scan-assembler-times "\\t(srai)" 128 { target rv64 } } } */
-/* { dg-final { scan-assembler-times "\\t(orn|ori|bset)" 128 { target rv64 } } } */
+/* { dg-final { scan-assembler-times "\\t(orn|ori|bset)" 196 { target rv64 } } } */
 
 /* { dg-final { scan-assembler-times "\\t(srai)" 64 { target rv32 } } } */
-/* { dg-final { scan-assembler-times "\\t(orn|ori|bset)" 64 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\t(orn|ori|bset)" 66 { target rv32 } } } */
 

diff --git a/gcc/testsuite/gcc.target/riscv/pr64345.c b/gcc/testsuite/gcc.target/riscv/pr64345.c
new file mode 100644 (file)
index 0000000..8ca4e24
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr64345.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gcbv_zicond -mabi=lp64d" { target rv64 } } */
+/* { dg-options "-O2 -march=rv32gcbv_zicond -mabi=ilp32" { target rv32 } } */
+
+
+
+unsigned int test0 (unsigned int x) { return ((x >> 4) ^ 1) & 1; }
+
+unsigned int test1 (unsigned int x) { return ((x >> 4) & 1) ^ 1; }
+
+unsigned int test2 (unsigned int x) { return ~(x >> 4) & 1; }
+
+unsigned int test3 (unsigned int x) { return ((~x >> 4) & 1); }
+
+unsigned int test4 (unsigned int x) { return (x >> 4) & 1; }
+
+int test5 (int vi) { return vi - (((vi >> 6) & 0x01) << 1); }
+
+int test6 (int vi) { return vi - (((vi >> 6) & 0x01) << 1) - 1; }
+
+
+/* { dg-final { scan-assembler-times "\\tbexti" 5 } } */
+/* { dg-final { scan-assembler-times "\\txori" 3 } } */
+/* { dg-final { scan-assembler-times "\\tnot" 1 } } */
+/* { dg-final { scan-assembler-times "\\tsrli" 2 } } */
+/* { dg-final { scan-assembler-times "\\tandi" 2 } } */
+/* { dg-final { scan-assembler-times "\\tsub" 2 } } */
+/* { dg-final { scan-assembler-times "\\taddi" 1 } } */
+
+

diff --git a/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-01.c b/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-01.c
index f9f32227bd58336dd6e0049ad324208b74940420..9d8a772fb8f30b2dbdbbfcf288fe69eb2799b3c3 100644 (file)
--- a/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-01.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-01.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-march=rv64gc_zbb -mabi=lp64" } */
-/* { dg-skip-if "" { *-*-* } { "-O0" "-g" "-Oz" "-Os" } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-g" "-O1" "-Oz" "-Os" } } */
 
 int foo1(int rs1)
 {

diff --git a/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-02.c b/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-02.c
index 112c0fa968eb6047bad9b196e6afd6aab66f527f..430d9984c4b3c7b0eb20c4aeb4093c13e76dcdd8 100644 (file)
--- a/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-02.c
+++ b/gcc/testsuite/gcc.target/riscv/zbb-andn-orn-02.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-march=rv32gc_zbb -mabi=ilp32" } */
-/* { dg-skip-if "" { *-*-* } { "-O0" "-g" "-Oz" "-Os" } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-g" "-O1" "-Oz" "-Os" } } */
 
 int foo1(int rs1)
 {