Fix gcc.dg/lower-subreg-1.c failure (was: [C Patch]: pr52543)

[Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>]

> From: Richard Sandiford <rdsandiford@googlemail.com>
> Date: Tue, 1 May 2012 16:46:38 +0200

> To repeat: as things stand, very few targets define proper rtx costs
> for SET.

IMHO it's wrong to start blaming targets when rtx_cost doesn't
take the mode in account in the first place, for the default
cost.  (Well, except for the modes-tieable subreg special-case.)
The targets where an operation in N * word_mode costs no more
than one in word_mode, if there even is one, is a minority,
let's adjust the defaults to that.

>  This patch is therefore expected to prevent lower-subreg
> from running in cases where it's actually benefical.  If you see that
> happening, please check whether the rtx_costs are defined properly.

Well, for CRIS (one of the targets of the PR53176 fallout) they
are sane, basically.  Where cris_rtx_costs returns true, it
returns mostly(*) ballparkly-correct costs *where it's passed an
rtx for which there's a corresponding insn*, otherwise falling
back to the defaults.  It shouldn't have to check for validity
of the rtx asked about; core GCC already knows which insns there
are and can gate that in rtx_cost or its callers.

(*) I see a bug in that cris_rtx_costs doesn't check the mode
for extendsidi2, to return COSTS_N_INSNS (3) instead of 0
(because a sign-extending move to SImode doesn't cost more than
a move; a sign- or zero-extension is also free in an operand for
addition and multiplication).  But this isn't on the path that
lower-subreg.c takes, so only an incidental observation...

> Of course, if the costs are defined properly and lower-subreg still
> makes the wrong choice, we need to look at why.

By the way, regarding validity of rtx_cost calls:

> +++ gcc/lower-subreg.c  2012-05-01 09:46:48.473830772 +0100

> +/* Return the cost of a CODE shift in mode MODE by OP1 bits, using the
> +   rtxes in RTXES.  SPEED_P selects between the speed and size cost.  */
> +
> +static int
> +shift_cost (bool speed_p, struct cost_rtxes *rtxes, enum rtx_code code,
> +           enum machine_mode mode, int op1)
> +{
> +  PUT_MODE (rtxes->target, mode);
> +  PUT_CODE (rtxes->shift, code);
> +  PUT_MODE (rtxes->shift, mode);
> +  PUT_MODE (rtxes->source, mode);
> +  XEXP (rtxes->shift, 1) = GEN_INT (op1);
> +  SET_SRC (rtxes->set) = rtxes->shift;
> +  return insn_rtx_cost (rtxes->set, speed_p);
> +}
> +
> +/* For each X in the range [0, BITS_PER_WORD), set SPLITTING[X]
> +   to true if it is profitable to split a double-word CODE shift
> +   of X + BITS_PER_WORD bits.  SPEED_P says whether we are testing
> +   for speed or size profitability.
> +
> +   Use the rtxes in RTXES to calculate costs.  WORD_MOVE_ZERO_COST is
> +   the cost of moving zero into a word-mode register.  WORD_MOVE_COST
> +   is the cost of moving between word registers.  */
> +
> +static void
> +compute_splitting_shift (bool speed_p, struct cost_rtxes *rtxes,
> +                        bool *splitting, enum rtx_code code,
> +                        int word_move_zero_cost, int word_move_cost)
> +{

I think there should be a gating check whether the target
implements that kind of shift in that mode at all, before
checking the cost.  Not sure whether it's generally best to put
that test here, or to make the rtx_cost function return the cost
of a libcall for that mode when that happens.  Similar for the
other insns.

Isn't the below better than doing virtually the same in each
target's rtx_costs?  Not tested yet besides "make cc1" and
checking that lower-subreg.c yields sane costs and that
gcc.dg/lower-subreg-1.c passes for cris-elf.  Note that
untieable SUBREGs still get a higher cost than tieable ones.

I'll test this for cris-elf, please tell me if/what other tests
and targets are required (simulator or compilefarm targets only,
please).

	* rtlanal.c (rtx_cost): Adjust default cost for X with a
	UNITS_PER_WORD factor for all X according to the size of
	its mode, not just for SUBREGs with untieable modes.

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	(revision 187308)
+++ gcc/rtlanal.c	(working copy)
@@ -3755,10 +3755,17 @@ rtx_cost (rtx x, enum rtx_code outer_cod
   enum rtx_code code;
   const char *fmt;
   int total;
+  int factor;
 
   if (x == 0)
     return 0;
 
+  /* A size N times larger than UNITS_PER_WORD likely needs N times as
+     many insns, taking N times as long.  */
+  factor = GET_MODE_SIZE (GET_MODE (x)) / UNITS_PER_WORD;
+  if (factor == 0)
+    factor = 1;
+
   /* Compute the default costs of certain things.
      Note that targetm.rtx_costs can override the defaults.  */
 
@@ -3766,20 +3773,27 @@ rtx_cost (rtx x, enum rtx_code outer_cod
   switch (code)
     {
     case MULT:
-      total = COSTS_N_INSNS (5);
+      total = factor * COSTS_N_INSNS (5);
       break;
     case DIV:
     case UDIV:
     case MOD:
     case UMOD:
-      total = COSTS_N_INSNS (7);
+      total = factor * COSTS_N_INSNS (7);
       break;
     case USE:
       /* Used in combine.c as a marker.  */
       total = 0;
       break;
+    case SET:
+      /* A SET doesn't have a mode, so let's look at the SET_DEST to get
+	 the mode for the factor.  */
+      factor = GET_MODE_SIZE (GET_MODE (SET_DEST (x))) / UNITS_PER_WORD;
+      if (factor == 0)
+	factor = 1;
+      /* Pass through.  */
     default:
-      total = COSTS_N_INSNS (1);
+      total = factor * COSTS_N_INSNS (1);
     }
 
   switch (code)
@@ -3792,8 +3806,7 @@ rtx_cost (rtx x, enum rtx_code outer_cod
       /* If we can't tie these modes, make this expensive.  The larger
 	 the mode, the more expensive it is.  */
       if (! MODES_TIEABLE_P (GET_MODE (x), GET_MODE (SUBREG_REG (x))))
-	return COSTS_N_INSNS (2
-			      + GET_MODE_SIZE (GET_MODE (x)) / UNITS_PER_WORD);
+	return COSTS_N_INSNS (2 + factor);
       break;
 
     default:

brgds, H-P