| Message ID | 20240122061239.83409-1-monk.chiang@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] RISC-V: Add split pattern to generate SFB instructions. [PR113095] | expand |
On 1/21/24 23:12, Monk Chiang wrote: > Since the match.pd transforms (zero_one == 0) ? y : z <op> y, > into ((typeof(y))zero_one * z) <op> y. Add splitters to recongize > this expression to generate SFB instructions. > > gcc/ChangeLog: > PR target/113095 > * config/riscv/sfb.md: New splitters to rewrite single bit > sign extension as the condition to SFB instructions. > > gcc/testsuite/ChangeLog: > * gcc.target/riscv/sfb.c: New test. > * gcc.target/riscv/pr113095.c: New test. So the 113095 test is going to fail to link on rv64 causing a testsuite failure. I would suggest it have these dg-options lines instead of the one you provided: /* { dg-options "-O2 -march=rv32gc -mabi=ilp32d -mtune=sifive-7-series" { target { rv32 } } } */ /* { dg-options "-O2 -march=rv64gc -mabi=lp64d -mtune=sifive-7-series" { target { rv64 } } } */ A similar change is not strictly needed for the new sfb.c test since it only does a compile (but not a link) test. You still didn't indicating what testing was done for this patch. Standard practice is to build the compiler and run the testsuite with and without your change and verify there are no regressions. Ideally new tests should pass as well. I made the change above locally to pr113095.c to fix those failures on rv64. So this is OK with the adjustment to the dg-options line in the new pr113095 test. Jeff
Thank you for your help. I will update the test case. I test on the Coremark and have 5% improvement on the SiFive CPU. On Tue, Jan 23, 2024 at 12:24 PM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > On 1/21/24 23:12, Monk Chiang wrote: > > Since the match.pd transforms (zero_one == 0) ? y : z <op> y, > > into ((typeof(y))zero_one * z) <op> y. Add splitters to recongize > > this expression to generate SFB instructions. > > > > gcc/ChangeLog: > > PR target/113095 > > * config/riscv/sfb.md: New splitters to rewrite single bit > > sign extension as the condition to SFB instructions. > > > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/sfb.c: New test. > > * gcc.target/riscv/pr113095.c: New test. > So the 113095 test is going to fail to link on rv64 causing a testsuite > failure. I would suggest it have these dg-options lines instead of the > one you provided: > > /* { dg-options "-O2 -march=rv32gc -mabi=ilp32d -mtune=sifive-7-series" > { target { rv32 } } } */ > /* { dg-options "-O2 -march=rv64gc -mabi=lp64d -mtune=sifive-7-series" { > target { rv64 } } } */ > > > A similar change is not strictly needed for the new sfb.c test since it > only does a compile (but not a link) test. > > You still didn't indicating what testing was done for this patch. > Standard practice is to build the compiler and run the testsuite with > and without your change and verify there are no regressions. Ideally > new tests should pass as well. > > I made the change above locally to pr113095.c to fix those failures on > rv64. So this is OK with the adjustment to the dg-options line in the > new pr113095 test. > > Jeff > >
diff --git a/gcc/config/riscv/sfb.md b/gcc/config/riscv/sfb.md index 8ab747142c8..bfd229e3d09 100644 --- a/gcc/config/riscv/sfb.md +++ b/gcc/config/riscv/sfb.md @@ -35,3 +35,35 @@ [(set_attr "length" "8") (set_attr "type" "sfb_alu") (set_attr "mode" "<GPR:MODE>")]) + +;; Combine creates this form ((typeof(y))zero_one * z) <op> y +;; for SiFive short forward branches. + +(define_split + [(set (match_operand:X 0 "register_operand") + (and:X (sign_extract:X (match_operand:X 1 "register_operand") + (const_int 1) + (match_operand 2 "immediate_operand")) + (match_operand:X 3 "register_operand"))) + (clobber (match_operand:X 4 "register_operand"))] + "TARGET_SFB_ALU" + [(set (match_dup 4) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2))) + (set (match_dup 0) (if_then_else:X (ne (match_dup 4) (const_int 0)) + (match_dup 3) + (const_int 0)))]) + +(define_split + [(set (match_operand:X 0 "register_operand") + (and:X (sign_extract:X (match_operand:X 1 "register_operand") + (const_int 1) + (match_operand 2 "immediate_operand")) + (match_operand:X 3 "register_operand"))) + (clobber (match_operand:X 4 "register_operand"))] + "TARGET_SFB_ALU && (UINTVAL (operands[2]) < 11)" + [(set (match_dup 4) (and:X (match_dup 1) (match_dup 2))) + (set (match_dup 0) (if_then_else:X (ne (match_dup 4) (const_int 0)) + (match_dup 3) + (const_int 0)))] +{ + operands[2] = GEN_INT (1 << UINTVAL(operands[2])); +}) diff --git a/gcc/testsuite/gcc.target/riscv/pr113095.c b/gcc/testsuite/gcc.target/riscv/pr113095.c new file mode 100644 index 00000000000..50dd6be11c4 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr113095.c @@ -0,0 +1,20 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -march=rv32gc -mabi=ilp32d -mtune=sifive-7-series" } */ + +extern void abort (void); +extern void exit (int); + +unsigned short __attribute__ ((noinline, noclone)) +foo (unsigned short x) { + if (x == 1) + x ^= 0x4002; + + return x; +} + +int main () { + if (foo(1) != 0x4003) + abort (); + + exit(0); +} diff --git a/gcc/testsuite/gcc.target/riscv/sfb.c b/gcc/testsuite/gcc.target/riscv/sfb.c new file mode 100644 index 00000000000..22f164051f4 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/sfb.c @@ -0,0 +1,24 @@ +//* { dg-do compile } */ +/* { dg-options "-O2 -march=rv32gc -mabi=ilp32d -mtune=sifive-7-series" } */ + +int f1(unsigned int x, unsigned int y, unsigned int z) +{ + return ((x & 1) == 0) ? y : z ^ y; +} + +int f2(unsigned int x, unsigned int y, unsigned int z) +{ + return ((x & 1) != 0) ? z ^ y : y; +} + +int f3(unsigned int x, unsigned int y, unsigned int z) +{ + return ((x & 1) == 0) ? y : z | y; +} + +int f4(unsigned int x, unsigned int y, unsigned int z) +{ + return ((x & 1) != 0) ? z | y : y; +} +/* { dg-final { scan-assembler-times "bne" 4 } } */ +/* { dg-final { scan-assembler-times "movcc" 4 } } */
Since the match.pd transforms (zero_one == 0) ? y : z <op> y, into ((typeof(y))zero_one * z) <op> y. Add splitters to recongize this expression to generate SFB instructions. gcc/ChangeLog: PR target/113095 * config/riscv/sfb.md: New splitters to rewrite single bit sign extension as the condition to SFB instructions. gcc/testsuite/ChangeLog: * gcc.target/riscv/sfb.c: New test. * gcc.target/riscv/pr113095.c: New test. --- gcc/config/riscv/sfb.md | 32 +++++++++++++++++++++++ gcc/testsuite/gcc.target/riscv/pr113095.c | 20 ++++++++++++++ gcc/testsuite/gcc.target/riscv/sfb.c | 24 +++++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/pr113095.c create mode 100644 gcc/testsuite/gcc.target/riscv/sfb.c