On Sun, 2016-09-25 at 16:06 +0900, Oleg Endo wrote: > > This fixes a fallout that actually goes back to 5.0 but went > unnoticed. The costs for movt and movrt type of insns were not > correctly reported and ifcvt thus made some bad choices for SH. A > new cset_zero pattern variant is also required to fix the matching > for some recent changes in the middle end. > > Tested on sh-elf with > make -k check RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,- > m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}" > > Committed as r240471. > Backports to GCC 6 and GCC 5 will follow. >
While working on the backport, I noticed that I've screwed up the costs return value. Funny that it had no effect in this case. Maybe rtx_costs functions should be made to return something like std::optional<int> to avoid these kind of mistakes. It would also cut the code by half in most cases. Anyway, fixed as attached, re-tested as above and committed as r240533. Cheers, Oleg gcc/ChangeLog PR target/51244 * config/sh/sh.c (sh_rtx_costs): Fix return value of SET of movt and movrt patterns. Match them before anything else in the SET case.
Index: gcc/config/sh/sh.c =================================================================== --- gcc/config/sh/sh.c (revision 240471) +++ gcc/config/sh/sh.c (working copy) @@ -3199,6 +3199,12 @@ vector-move, so we have to provide the correct cost in the number of move insns to load/store the reg of the mode in question. */ case SET: + if (sh_movt_set_dest (x) != NULL || sh_movrt_set_dest (x) != NULL) + { + *total = COSTS_N_INSNS (1); + return true; + } + if (register_operand (SET_DEST (x), VOIDmode) && (register_operand (SET_SRC (x), VOIDmode) || satisfies_constraint_Z (SET_SRC (x)))) @@ -3208,10 +3214,6 @@ / mov_insn_size (mode, TARGET_SH2A)); return true; } - - if (sh_movt_set_dest (x) != NULL || sh_movrt_set_dest (x) != NULL) - return COSTS_N_INSNS (1); - return false; /* The cost of a mem access is mainly the cost of the address mode. */