diff options
author | Jeff Law <jlaw@ventanamicro.com> | 2024-09-05 15:45:25 -0600 |
---|---|---|
committer | Jeff Law <jlaw@ventanamicro.com> | 2024-09-05 15:45:25 -0600 |
commit | b567e5ead5d54f022c57b48f31653f6ae6ece007 (patch) | |
tree | e3f8f7e9e241767d3f5e844c1014319f51f3de1b /gcc/config | |
parent | b214ffeecb3f480dd7864bcbfc9ac30100c126f3 (diff) | |
download | gcc-b567e5ead5d54f022c57b48f31653f6ae6ece007.zip gcc-b567e5ead5d54f022c57b48f31653f6ae6ece007.tar.gz gcc-b567e5ead5d54f022c57b48f31653f6ae6ece007.tar.bz2 |
[V2][RISC-V] Avoid unnecessary extensions after sCC insns
So the first patch failed the pre-commit CI; it didn't fail in my testing
because I'm using --with-arch to set a default configuration that includes
things like zicond to ensure that's always tested. And the failing test is
skipped when zicond is enabled by default.
The failing test is designed to ensure that we don't miss an if-conversion due
to costing issues around the extension that was typically done in an sCC
sequence (which is why it's only run when zicond is off).
The test failed because we have a little routine that is highly dependent on
the code generated by the sCC expander and will adjust the costing to account
for expansion quirks that usually go away in register allocation.
That code needs to be enhanced to work after the sCC expansion change.
Essentially it needs to account for the subreg extraction that shows up in the
sequence as well as being a bit looser on mode checking.
I kept the code working for the old sequences -- in theory a user could conjure
up the old sequence so handling them seems useful.
This also drops the testsuite changes. Palmer's change makes them unnecessary.
---
So I was looking at a performance regression in spec with Ventana's internal
tree. Ultimately the problem was a bad interaction with an internal patch
(REP_MODE_EXTENDED), fwprop and ext-dce. The details of that problem aren't
particularly important.
Removal of the local patch went reasonably well. But I did see some secondary
cases where we had redundant sign extensions. The most notable cases come from
the integer sCC insns.
Expansion of those cases for rv64 can be improved using Jivan's trick. ie, if
the target is not DImode, then create a DImode temporary for the result and
copy the low bits out with a promoted subreg to the real target.
With the change in expansion the final code we generate is slightly different
for a few tests at -O1/-Og, but should perform the same. The key for the
affected tests is we're not seeing the introduction of unnecessary extensions.
Rather than adjust the regexps to handle the -O1/-Og output, skipping for those
seemed OK to me. I didn't extract a testcase. I'm a bit fried from digging
through LTO'd code right now.
gcc/
* config/riscv/riscv.cc (riscv_expand_int_scc): For rv64, use a DI
temporary for the output and a promoted subreg to extract it into SI
arget.
(riscv_noce_conversion_profitable_p): Recognize new output from
sCC expansion too.
Diffstat (limited to 'gcc/config')
-rw-r--r-- | gcc/config/riscv/riscv.cc | 46 |
1 files changed, 41 insertions, 5 deletions
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index a38cb72..39489c4 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -4218,11 +4218,29 @@ 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); } - last_dest = NULL_RTX; + rtx dest = SET_DEST (x); - if (COMPARISON_P (src) + + /* Do something similar for the moves that are likely to + turn into NOP moves by the time the register allocator is + done. These are also side effects of how our sCC expanders + work. We'll want to check and update LAST_DEST here too. */ + if (last_dest && REG_P (dest) - && GET_MODE (dest) == SImode) + && GET_MODE (dest) == SImode + && SUBREG_P (src) + && SUBREG_PROMOTED_VAR_P (src) + && REGNO (SUBREG_REG (src)) == REGNO (last_dest)) + { + riscv_if_info.original_cost += COSTS_N_INSNS (1); + riscv_if_info.max_seq_cost += COSTS_N_INSNS (1); + if (last_dest) + last_dest = dest; + } + else + last_dest = NULL_RTX; + + if (COMPARISON_P (src) && REG_P (dest)) last_dest = dest; } else @@ -4904,13 +4922,31 @@ riscv_expand_int_scc (rtx target, enum rtx_code code, rtx op0, rtx op1, bool *in riscv_extend_comparands (code, &op0, &op1); op0 = force_reg (word_mode, op0); + /* For sub-word targets on rv64, do the computation in DImode + then extract the lowpart for the final target, marking it + as sign extended. Note that it's also properly zero extended, + but it's probably more profitable to expose it as sign extended. */ + rtx t; + if (TARGET_64BIT && GET_MODE (target) == SImode) + t = gen_reg_rtx (DImode); + else + t = target; + if (code == EQ || code == NE) { rtx zie = riscv_zero_if_equal (op0, op1); - riscv_emit_binary (code, target, zie, const0_rtx); + riscv_emit_binary (code, t, zie, const0_rtx); } else - riscv_emit_int_order_test (code, invert_ptr, target, op0, op1); + riscv_emit_int_order_test (code, invert_ptr, t, op0, op1); + + if (t != target) + { + t = gen_lowpart (SImode, t); + SUBREG_PROMOTED_VAR_P (t) = 1; + SUBREG_PROMOTED_SET (t, SRP_SIGNED); + emit_move_insn (target, t); + } } /* Like riscv_expand_int_scc, but for floating-point comparisons. */ |