diff mbox

[SH,committed] Fix cset_zero pattern regressions

Message ID 1474981008.23880.6.camel@t-online.de
State New
Headers show

Commit Message

Oleg Endo Sept. 27, 2016, 12:56 p.m. UTC 
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.
diff mbox

Patch

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.  */