2016-09-22 7:52 GMT+03:00 Senthil Kumar Selvaraj
<senthil_kumar.selva...@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.selva...@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))
