Message ID | alpine.DEB.2.20.2311180424040.5892@tpp.orcam.me.uk |
---|---|
State | New |
Headers | show |
Series | RISC-V: Various if-conversion fixes and improvements | expand |
On 11/18/23 22:38, Maciej W. Rozycki wrote: > In the non-zero case there is no need for the conditional value used by > Ventana and Zicond integer conditional operations to be specifically 1. > Regardless we canonicalize it by producing an extraneous conditional-set > operation, such as with the sequence below: > > (insn 22 6 23 2 (set (reg:DI 141) > (minus:DI (reg/v:DI 135 [ w ]) > (reg/v:DI 136 [ x ]))) 11 {subdi3} > (nil)) > (insn 23 22 24 2 (set (reg:DI 140) > (ne:DI (reg:DI 141) > (const_int 0 [0]))) 307 {*sne_zero_didi} > (nil)) > (insn 24 23 25 2 (set (reg:DI 143) > (if_then_else:DI (eq:DI (reg:DI 140) > (const_int 0 [0])) > (const_int 0 [0]) > (reg:DI 13 a3 [ z ]))) 27913 {*czero.eqz.didi} > (nil)) > (insn 25 24 26 2 (set (reg:DI 142) > (if_then_else:DI (ne:DI (reg:DI 140) > (const_int 0 [0])) > (const_int 0 [0]) > (reg/v:DI 137 [ y ]))) 27914 {*czero.nez.didi} > (nil)) > (insn 26 25 18 2 (set (reg/v:DI 138 [ z ]) > (ior:DI (reg:DI 142) > (reg:DI 143))) 105 {iordi3} > (nil)) > > where insn 23 can well be removed without changing the semantics of the > sequence. This is actually fixed up later on by combine and the insn > does not make it to output meaning no SNEZ (or SEQZ in the reverse case) > appears in the assembly produced, however it counts towards the cost of > the sequence calculated by if-conversion, raising the trigger level for > the branchless sequence to be chosen. Arguably to emit this extraneous > operation it can be also considered rather sloppy of our backend's. > > Remove the check for operand 1 being constant 0 in the Ventana/Zicond > case for equality comparisons then, observing that `riscv_zero_if_equal' > called via `riscv_emit_int_compare' will canonicalize the comparison if > required, removing the extraneous insn from output: > > (insn 22 6 23 2 (set (reg:DI 142) > (minus:DI (reg/v:DI 135 [ w ]) > (reg/v:DI 136 [ x ]))) 11 {subdi3} > (nil)) > (insn 23 22 24 2 (set (reg:DI 141) > (if_then_else:DI (eq:DI (reg:DI 142) > (const_int 0 [0])) > (const_int 0 [0]) > (reg:DI 13 a3 [ z ]))) 27913 {*czero.eqz.didi} > (nil)) > (insn 24 23 25 2 (set (reg:DI 140) > (if_then_else:DI (ne:DI (reg:DI 142) > (const_int 0 [0])) > (const_int 0 [0]) > (reg/v:DI 137 [ y ]))) 27914 {*czero.nez.didi} > (nil)) > (insn 25 24 18 2 (set (reg/v:DI 138 [ z ]) > (ior:DI (reg:DI 140) > (reg:DI 141))) 105 {iordi3} > (nil)) > > while keeping actual assembly produced the same. > > Adjust branch costs across the test cases affected accordingly. > > gcc/ > * config/riscv/riscv.cc (riscv_expand_conditional_move): Remove > the check for operand 1 being constant 0 in the Ventana/Zicond > case for equality comparisons. > > gcc/testsuite/ > * gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_imm.c: > Lower `-mbranch-cost=' setting. > * gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_reg.c: > Likewise. > * gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_reg_reg.c: > Likewise. > * gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_imm.c: > Likewise. > * gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_reg.c: > Likewise. > * gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_reg_reg.c: > Likewise. OK. Thanks for catching this! jeff
Index: gcc/gcc/config/riscv/riscv.cc =================================================================== --- gcc.orig/gcc/config/riscv/riscv.cc +++ gcc/gcc/config/riscv/riscv.cc @@ -4132,9 +4132,9 @@ riscv_expand_conditional_move (rtx dest, return false; /* Canonicalize the comparison. It must be an equality comparison - against 0. If it isn't, then emit an SCC instruction so that - we can then use an equality comparison against zero. */ - if (!equality_operator (op, VOIDmode) || op1 != CONST0_RTX (mode)) + of integer operands. If it isn't, then emit an SCC instruction + so that we can then use an equality comparison against zero. */ + if (!equality_operator (op, VOIDmode) || !INTEGRAL_MODE_P (mode0)) { bool *invert_ptr = nullptr; bool invert = false; Index: gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_imm.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_imm.c +++ gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_imm.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=4" { target { rv64 } } } */ -/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=4" { target { rv32 } } } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=3" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=3" { target { rv32 } } } */ /* { dg-skip-if "" { *-*-* } {"-O0" "-Og" "-Os" "-Oz"} } */ long primitiveSemantics_compare_imm_return_imm_imm_00(long a, long b) { Index: gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_reg.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_reg.c +++ gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_imm_reg.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=4" { target { rv64 } } } */ -/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=4" { target { rv32 } } } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=3" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=3" { target { rv32 } } } */ /* { dg-skip-if "" { *-*-* } {"-O0" "-Og" "-Os" "-Oz"} } */ long primitiveSemantics_compare_imm_return_imm_reg_00(long a, long b) { Index: gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_reg_reg.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_reg_reg.c +++ gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_imm_return_reg_reg.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=5" { target { rv64 } } } */ -/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=5" { target { rv32 } } } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=4" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=4" { target { rv32 } } } */ /* { dg-skip-if "" { *-*-* } {"-O0" "-Og" "-Os" "-Oz"} } */ long primitiveSemantics_compare_imm_return_reg_reg_00(long a, long b, long c) { Index: gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_imm.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_imm.c +++ gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_imm.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=4" { target { rv64 } } } */ -/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=4" { target { rv32 } } } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=3" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=3" { target { rv32 } } } */ /* { dg-skip-if "" { *-*-* } {"-O0" "-Og" "-Os" "-Oz"} } */ long primitiveSemantics_compare_reg_return_imm_imm_00(long a, long b, long c) { Index: gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_reg.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_reg.c +++ gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_imm_reg.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=4" { target { rv64 } } } */ -/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=4" { target { rv32 } } } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=3" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=3" { target { rv32 } } } */ /* { dg-skip-if "" { *-*-* } {"-O0" "-Og" "-Os" "-Oz"} } */ long primitiveSemantics_compare_reg_return_imm_reg_00(long a, long b, long c) { Index: gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_reg_reg.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_reg_reg.c +++ gcc/gcc/testsuite/gcc.target/riscv/zicond-primitiveSemantics_compare_reg_return_reg_reg.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=5" { target { rv64 } } } */ -/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=5" { target { rv32 } } } */ +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d -mbranch-cost=4" { target { rv64 } } } */ +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32f -mbranch-cost=4" { target { rv32 } } } */ /* { dg-skip-if "" { *-*-* } {"-O0" "-Og" "-Os" "-Oz"} } */ long primitiveSemantics_compare_reg_return_reg_reg_00(long a, long b, long c,