* [Patch, avr] Provide correct LD offset bound in avr_address_cost
@ 2016-09-22 6:45 Senthil Kumar Selvaraj
2016-09-22 7:41 ` Denis Chertykov
0 siblings, 1 reply; 2+ messages in thread
From: Senthil Kumar Selvaraj @ 2016-09-22 6:45 UTC (permalink / raw)
To: gcc-patches; +Cc: Denis Chertykov
Hi,
This patch fixes cost computation in avr_address_cost - instead of the
hardcoded 61, it uses the already existing MAX_LD_OFFSET(mode) macro.
This showed up when investigating a code size regression in the ivopts
pass. That pass computes address_cost with and without an offset to
decide on the right induction variable candidate(s). The legitimate
address target hook returns false for offsets more than 63, so the
pass calls the TARGET_ADDRESS_COST hook with 62 as the offset.
The hook implementation returns 18, and the ivopts pass concludes that
the cost of address with *any* offset is 18, which is not true - the higher
cost is incurred only with offsets bigger than MAX_LD_OFFSET. This in
turn results in a suboptimal choice of induction variables in the
ivopts pass. The patch changes the hardcoded 61 to use the mode
specific MAX_LD_OFFSET instead.
Regression testing with just that fix showed one additional
compilation timeout. That turned out to be the same as
https://lists.nongnu.org/archive/html/avr-gcc-list/2014-03/msg00010.html
- the middle end takes too much time to decide on the best strategy to
multiply DImode values on a 64 bit host. This already causes timeouts
for a few builtin-arith-overflow-* tests (see
https://gcc.gnu.org/ml/gcc-testresults/2016-09/msg02018.html), so it
isn't really related to this fix. Just providing a cost estimate for
DImode mul fixes the timeout though, so the patch does that by scaling
SImode costs by 2 for DImode muls.
With both changes in, there are no regressions, and the
builtin-arith-overflow-* tests now PASS and don't timeout.
Ok to commit to trunk and backport to 5.x?
Regards
Senthil
gcc/ChangeLog:
2016-09-21 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com>
* config/avr/avr.c (avr_rtx_costs_1): Handle DImode MULT.
(avr_address_cost): Replace 61 with MAX_LD_OFFSET(mode).
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 148a61d..29f0022 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -10257,6 +10257,7 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int outer_code ATTRIBUTE_UNUSED,
break;
case SImode:
+ case DImode:
if (AVR_HAVE_MUL)
{
if (!speed)
@@ -10282,7 +10283,10 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int outer_code ATTRIBUTE_UNUSED,
*total = COSTS_N_INSNS (AVR_HAVE_JMP_CALL ? 5 : 4);
}
- return true;
+ if (mode == DImode)
+ *total *= 2;
+
+ return true;
default:
return false;
@@ -10863,7 +10867,7 @@ avr_address_cost (rtx x, machine_mode mode ATTRIBUTE_UNUSED,
&& (REG_P (XEXP (x, 0))
|| GET_CODE (XEXP (x, 0)) == SUBREG))
{
- if (INTVAL (XEXP (x, 1)) >= 61)
+ if (INTVAL (XEXP (x, 1)) > MAX_LD_OFFSET(mode))
cost = 18;
}
else if (CONSTANT_ADDRESS_P (x))
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Patch, avr] Provide correct LD offset bound in avr_address_cost
2016-09-22 6:45 [Patch, avr] Provide correct LD offset bound in avr_address_cost Senthil Kumar Selvaraj
@ 2016-09-22 7:41 ` Denis Chertykov
0 siblings, 0 replies; 2+ messages in thread
From: Denis Chertykov @ 2016-09-22 7:41 UTC (permalink / raw)
To: Senthil Kumar Selvaraj; +Cc: gcc-patches
2016-09-22 7:52 GMT+03:00 Senthil Kumar Selvaraj
<senthil_kumar.selvaraj@atmel.com>:
> Hi,
>
> This patch fixes cost computation in avr_address_cost - instead of the
> hardcoded 61, it uses the already existing MAX_LD_OFFSET(mode) macro.
>
> This showed up when investigating a code size regression in the ivopts
> pass. That pass computes address_cost with and without an offset to
> decide on the right induction variable candidate(s). The legitimate
> address target hook returns false for offsets more than 63, so the
> pass calls the TARGET_ADDRESS_COST hook with 62 as the offset.
>
> The hook implementation returns 18, and the ivopts pass concludes that
> the cost of address with *any* offset is 18, which is not true - the higher
> cost is incurred only with offsets bigger than MAX_LD_OFFSET. This in
> turn results in a suboptimal choice of induction variables in the
> ivopts pass. The patch changes the hardcoded 61 to use the mode
> specific MAX_LD_OFFSET instead.
>
> Regression testing with just that fix showed one additional
> compilation timeout. That turned out to be the same as
> https://lists.nongnu.org/archive/html/avr-gcc-list/2014-03/msg00010.html
> - the middle end takes too much time to decide on the best strategy to
> multiply DImode values on a 64 bit host. This already causes timeouts
> for a few builtin-arith-overflow-* tests (see
> https://gcc.gnu.org/ml/gcc-testresults/2016-09/msg02018.html), so it
> isn't really related to this fix. Just providing a cost estimate for
> DImode mul fixes the timeout though, so the patch does that by scaling
> SImode costs by 2 for DImode muls.
>
> With both changes in, there are no regressions, and the
> builtin-arith-overflow-* tests now PASS and don't timeout.
>
> Ok to commit to trunk and backport to 5.x?
A deep analysis...
Approved.
>
> Regards
> Senthil
>
>
> gcc/ChangeLog:
>
> 2016-09-21 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com>
>
> * config/avr/avr.c (avr_rtx_costs_1): Handle DImode MULT.
> (avr_address_cost): Replace 61 with MAX_LD_OFFSET(mode).
>
>
>
> diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
> index 148a61d..29f0022 100644
> --- gcc/config/avr/avr.c
> +++ gcc/config/avr/avr.c
> @@ -10257,6 +10257,7 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int outer_code ATTRIBUTE_UNUSED,
> break;
>
> case SImode:
> + case DImode:
> if (AVR_HAVE_MUL)
> {
> if (!speed)
> @@ -10282,7 +10283,10 @@ avr_rtx_costs_1 (rtx x, machine_mode mode, int outer_code ATTRIBUTE_UNUSED,
> *total = COSTS_N_INSNS (AVR_HAVE_JMP_CALL ? 5 : 4);
> }
>
> - return true;
> + if (mode == DImode)
> + *total *= 2;
> +
> + return true;
>
> default:
> return false;
> @@ -10863,7 +10867,7 @@ avr_address_cost (rtx x, machine_mode mode ATTRIBUTE_UNUSED,
> && (REG_P (XEXP (x, 0))
> || GET_CODE (XEXP (x, 0)) == SUBREG))
> {
> - if (INTVAL (XEXP (x, 1)) >= 61)
> + if (INTVAL (XEXP (x, 1)) > MAX_LD_OFFSET(mode))
> cost = 18;
> }
> else if (CONSTANT_ADDRESS_P (x))
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-09-22 7:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22 6:45 [Patch, avr] Provide correct LD offset bound in avr_address_cost Senthil Kumar Selvaraj
2016-09-22 7:41 ` Denis Chertykov
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).