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))