On Wed, May 22, 2024 at 5:15 PM Roger Sayle <ro...@nextmovesoftware.com> wrote: > > This single line patch fixes a strange quirk/glitch in i386's rtx_costs, > which considers an instruction loading a 64-bit constant to be significantly > cheaper than loading a 32-bit (or smaller) constant. > > Consider the two functions: > unsigned long long foo() { return 0x0123456789abcdefULL; } > unsigned int bar() { return 10; } > > and the corresponding lines from combine's dump file: > insn_cost 1 for #: r98:DI=0x123456789abcdef > insn_cost 4 for #: ax:SI=0xa > > The same issue can be seen in -dP assembler output. > movabsq $81985529216486895, %rax # 5 [c=1 l=10] *movdi_internal/4 > > The problem is that pattern_costs interpretation of rtx_costs contains > "return cost > 0 ? cost : COSTS_N_INSNS (1)" where a zero value (for > example a register or small immediate constant) is considered special, > and equivalent to a single instruction, but all other values are treated > as verbatim. Hence to make x86_64's 10-byte long movabsq instruction > slightly more expensive than a simple constant, rtx_costs needs to > return COSTS_N_INSNS(1)+1 and not 1. With this change, the insn_cost > of movabsq is the intended value 5: > insn_cost 5 for #: r98:DI=0x123456789abcdef > and > movabsq $81985529216486895, %rax # 5 [c=5 l=10] *movdi_internal/4 > > > [I'd originally tried fixing this by adding a ix86_insn_cost target > hook, but the testsuite is very sensitive to the costing of insns]. > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check, both with and without --target_board=unix{-m32} > with no new failures. Ok for mainline? > > > 2024-05-22 Roger Sayle <ro...@nextmovesoftware.com> > > gcc/ChangeLog > * config/i386/i386.cc (ix86_rtx_costs) <case CONST_INT>: > A CONST_INT that isn't x86_64_immediate_operand requires an extra > (expensive) movabsq insn to load, so return COSTS_N_INSNS (1) + 1.
1 of 20,796 [x86_64 PATCH] Correct insn_cost of movabsq. Inbox Roger Sayle 5:15 PM (12 minutes ago) to gcc-patches, me This single line patch fixes a strange quirk/glitch in i386's rtx_costs, which considers an instruction loading a 64-bit constant to be significantly cheaper than loading a 32-bit (or smaller) constant. Consider the two functions: unsigned long long foo() { return 0x0123456789abcdefULL; } unsigned int bar() { return 10; } and the corresponding lines from combine's dump file: insn_cost 1 for #: r98:DI=0x123456789abcdef insn_cost 4 for #: ax:SI=0xa The same issue can be seen in -dP assembler output. movabsq $81985529216486895, %rax # 5 [c=1 l=10] *movdi_internal/4 The problem is that pattern_costs interpretation of rtx_costs contains "return cost > 0 ? cost : COSTS_N_INSNS (1)" where a zero value (for example a register or small immediate constant) is considered special, and equivalent to a single instruction, but all other values are treated as verbatim. Hence to make x86_64's 10-byte long movabsq instruction slightly more expensive than a simple constant, rtx_costs needs to return COSTS_N_INSNS(1)+1 and not 1. With this change, the insn_cost of movabsq is the intended value 5: insn_cost 5 for #: r98:DI=0x123456789abcdef and movabsq $81985529216486895, %rax # 5 [c=5 l=10] *movdi_internal/4 [I'd originally tried fixing this by adding a ix86_insn_cost target hook, but the testsuite is very sensitive to the costing of insns]. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=unix{-m32} with no new failures. Ok for mainline? 2024-05-22 Roger Sayle <ro...@nextmovesoftware.com> gcc/ChangeLog * config/i386/i386.cc (ix86_rtx_costs) <case CONST_INT>: A CONST_INT that isn't x86_64_immediate_operand requires an extra (expensive) movabsq insn to load, so return COSTS_N_INSNS (1) + 1. Thanks in advance, Roger -- One attachment • Scanned by Gmail Roger Sayle (nextmovesoftware.com), gcc-patches@gcc.gnu.org On Wed, May 22, 2024 at 5:15 PM Roger Sayle <ro...@nextmovesoftware.com> wrote: > > This single line patch fixes a strange quirk/glitch in i386's rtx_costs, > which considers an instruction loading a 64-bit constant to be significantly > cheaper than loading a 32-bit (or smaller) constant. > > Consider the two functions: > unsigned long long foo() { return 0x0123456789abcdefULL; } > unsigned int bar() { return 10; } > > and the corresponding lines from combine's dump file: > insn_cost 1 for #: r98:DI=0x123456789abcdef > insn_cost 4 for #: ax:SI=0xa > > The same issue can be seen in -dP assembler output. > movabsq $81985529216486895, %rax # 5 [c=1 l=10] *movdi_internal/4 > > The problem is that pattern_costs interpretation of rtx_costs contains > "return cost > 0 ? cost : COSTS_N_INSNS (1)" where a zero value (for > example a register or small immediate constant) is considered special, > and equivalent to a single instruction, but all other values are treated > as verbatim. Hence to make x86_64's 10-byte long movabsq instruction > slightly more expensive than a simple constant, rtx_costs needs to > return COSTS_N_INSNS(1)+1 and not 1. With this change, the insn_cost > of movabsq is the intended value 5: > insn_cost 5 for #: r98:DI=0x123456789abcdef > and > movabsq $81985529216486895, %rax # 5 [c=5 l=10] *movdi_internal/4 > > > [I'd originally tried fixing this by adding a ix86_insn_cost target > hook, but the testsuite is very sensitive to the costing of insns]. > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check, both with and without --target_board=unix{-m32} > with no new failures. Ok for mainline? > > > 2024-05-22 Roger Sayle <ro...@nextmovesoftware.com> > > gcc/ChangeLog > * config/i386/i386.cc (ix86_rtx_costs) <case CONST_INT>: > A CONST_INT that isn't x86_64_immediate_operand requires an extra > (expensive) movabsq insn to load, so return COSTS_N_INSNS (1) + 1. OK, with a small comment added. Thanks, Uros. > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > index b4838b7..b4a9519 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -21569,7 +21569,7 @@ ix86_rtx_costs (rtx x, machine_mode mode, int > outer_code_i, int opno, > if (x86_64_immediate_operand (x, VOIDmode)) > *total = 0; > else > - *total = 1; > + *total = COSTS_N_INSNS (1) + 1; > return true; Please add a small comment that this cost belongs to movabs.