So I'm not happy with this entire routine, but after trying to fix
things right, I'm back to adjusting the cost routine instead.
The fundamental problem as I see it is the cstore patterns on RISC-V are
suboptimal.
Those patterns always have an SI destination. One expander is used for
integer inputs, the other for FP inputs.
In both the integer and FP case we use an iterator to allow different
input modes. But the destination stays fixed in SI.
But it'd be really advantageous to support an output iterator. With the
forced SI output we typically end up with subreg copies, zero extensions
and moves in sequences involving SCC insns. That is usually cleaned up
by combine, but those extra insns participate in if-conversion costing
and sometimes cause us to miss opportunities or force us into a
slower/bigger sequence using generalized conditional moves.
Of course the cstore only has one mode, which if I read the docs
correctly *should* be the output mode. RISC-V is using it for the input
modes. So we're doing something a bit dumb there. To achieve that we'd
need to combine the two expanders since they both have integer output
modes and we can't have two expanders with the same name. To combine
the expanders the inputs have to accept both integer and FP modes.
That's likely possible but it's going to get ugly (for example, use
modeless operands and check modes and FAIL in the expansion code).
So rather than fight that battle, this patch just ignores the trivial
copies that can be generated during expansion. The code was already
ignoring certain extensions and promoted subreg copies. With some in
flight if-conversion work we're able to take this testcase (adjusted
from a loongarch test):
extern long lm, ln, lr;
void
test_ne ()
{
if (lm != ln)
lr += (1 << 2);
lr += lm;
}
Which previously generated this code for the conditional add
beq a5,a2,.L2
addi a4,a4,4
.L2:
add a5,a5,a4
With the in flight if-converter work alone we'd generate:
sub t2,t0,t1
li a1,4
czero.eqz a0,a1,t2
add a6,a3,a0
add a7,t0,a6
It's straightline code and probably better due to that alone if the
branch has meaningful unpredictability for the hardware. Add in this
patch to send us through a different if-conversion path and we get:
sub t2,t0,t1
snez a0,t2
sh2add a1,a0,a2
add a6,t0,a1
For shift counts 1..3 we'll get that shNadd form. For shift counts
4..11 we'll get sll+add. For shift counts > 11 the middle block in the
if-conversion path has two insns because the constant can't encode into
an addi instruction and we can't use the specialized ifcvt paths.
Tested on riscv32-elf and riscv64-elf. Bootstrap & regression test on
k3 is in flight. Bootstrap and regression test and k1 and c920 will
fire up later today. Obviously I'll wait for pre-commit CI to render
its verdict, I expect I'll have k3 data by then as well.
jeff
gcc/
* config/riscv/riscv.cc (riscv_noce_conversion_profitable_p): Ignore
reg->reg copies too.
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 873defc5a5b5..3b150d34893d 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -5200,6 +5200,12 @@ riscv_noce_conversion_profitable_p (rtx_insn *seq,
riscv_if_info.original_cost += COSTS_N_INSNS (1);
riscv_if_info.max_seq_cost += COSTS_N_INSNS (1);
}
+ else if (REG_P (src) && REG_P (dest))
+ {
+ /* Trivial copies likely just get propagated away. */
+ riscv_if_info.original_cost += COSTS_N_INSNS (1);
+ riscv_if_info.max_seq_cost += COSTS_N_INSNS (1);
+ }
else
last_dest = NULL_RTX;
