* [x86_64 PATCH] Correct insn_cost of movabsq.
@ 2024-05-22 15:15 Roger Sayle
2024-05-22 15:29 ` Uros Bizjak
0 siblings, 1 reply; 3+ messages in thread
From: Roger Sayle @ 2024-05-22 15:15 UTC (permalink / raw)
To: gcc-patches; +Cc: 'Uros Bizjak'
[-- Attachment #1: Type: text/plain, Size: 1879 bytes --]
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 <roger@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
--
[-- Attachment #2: patchic3.txt --]
[-- Type: text/plain, Size: 430 bytes --]
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;
case CONST_DOUBLE:
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [x86_64 PATCH] Correct insn_cost of movabsq.
2024-05-22 15:15 [x86_64 PATCH] Correct insn_cost of movabsq Roger Sayle
@ 2024-05-22 15:29 ` Uros Bizjak
2024-05-22 17:54 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Uros Bizjak @ 2024-05-22 15:29 UTC (permalink / raw)
To: Roger Sayle; +Cc: gcc-patches
On Wed, May 22, 2024 at 5:15 PM Roger Sayle <roger@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 <roger@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 <roger@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 <roger@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 <roger@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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [x86_64 PATCH] Correct insn_cost of movabsq.
2024-05-22 15:29 ` Uros Bizjak
@ 2024-05-22 17:54 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2024-05-22 17:54 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Roger Sayle, gcc-patches
> Am 22.05.2024 um 17:30 schrieb Uros Bizjak <ubizjak@gmail.com>:
>
> On Wed, May 22, 2024 at 5:15 PM Roger Sayle <roger@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.
A zero cost is interpreted as „not implemented“ and assigned a cost of 1, assuming a COSTS_N_INSNS basing.
IMO a bit bogus but I didn’t dare to argue further with Segher.
Richard
>> 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 <roger@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 <roger@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 <roger@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 <roger@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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-22 17:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-22 15:15 [x86_64 PATCH] Correct insn_cost of movabsq Roger Sayle
2024-05-22 15:29 ` Uros Bizjak
2024-05-22 17:54 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).