diff mbox

[committed] Fix lower-subreg cost calculation

Message ID 87r4uxdtqr.fsf_-_@talisman.home
State New
Headers show

Commit Message

Richard Sandiford May 6, 2012, 6:55 p.m. UTC 
Georg-Johann Lay <avr@gjlay.de> writes:
> TARGET_RTX_COSTS gets called with x = (const_int 1) and outer = SET
> for example. How do I get SET_DEST from that information?
>
> I don't now if lower-subreg.s ever emits such cost requests, but several
> passes definitely do.

Gah!  I really should have remembered that insn_rtx_cost happily ignores
both SETs and SET_DESTs, and skips straight to the SET_SRC.  This caught
me out when looking at the auto-inc-dec rewrite last year too.  (The problem
in that case was that insn_rtx_cost ignored the cost of MEMs in stores,
and only took into account the cost of MEMs in loads.)

While that probably ought to change, I felt like I was going down a
rathole last time I looked at it, so this patch does what I should
have done originally.

For the record: I wondered whether rtlanal.c should base the default
register-to-register copy cost for mode M on the lowest move_cost[M][c][c].
The problem is that move_cost has traditionally been used to choose
between difference classes in the same mode, rather than between modes,
with 2 as the base cost.  So I don't think it's suitable.

Tested on x86_64-linux-gnu and with the upcoming MIPS costs.  Installed.

Sorry for the breakage.

Richard


gcc/
	* lower-subreg.c (shift_cost): Use set_src_cost, avoiding the SET.
	(compute_costs): Likewise for the zero extension.  Use set_rtx_cost
	to compute the cost of moves.  Set the mode of the target register.

Comments

Richard Earnshaw May 8, 2012, 3:26 p.m. UTC | #1 
On 06/05/12 19:55, Richard Sandiford wrote:
> Georg-Johann Lay <avr@gjlay.de> writes:
>> TARGET_RTX_COSTS gets called with x = (const_int 1) and outer = SET
>> for example. How do I get SET_DEST from that information?
>>
>> I don't now if lower-subreg.s ever emits such cost requests, but several
>> passes definitely do.
> 
> Gah!  I really should have remembered that insn_rtx_cost happily ignores
> both SETs and SET_DESTs, and skips straight to the SET_SRC.  This caught
> me out when looking at the auto-inc-dec rewrite last year too.  (The problem
> in that case was that insn_rtx_cost ignored the cost of MEMs in stores,
> and only took into account the cost of MEMs in loads.)
> 
> While that probably ought to change, I felt like I was going down a
> rathole last time I looked at it, so this patch does what I should
> have done originally.
> 
> For the record: I wondered whether rtlanal.c should base the default
> register-to-register copy cost for mode M on the lowest move_cost[M][c][c].
> The problem is that move_cost has traditionally been used to choose
> between difference classes in the same mode, rather than between modes,
> with 2 as the base cost.  So I don't think it's suitable.
> 
> Tested on x86_64-linux-gnu and with the upcoming MIPS costs.  Installed.
> 
> Sorry for the breakage.
> 
> Richard
> 
> 
> gcc/
> 	* lower-subreg.c (shift_cost): Use set_src_cost, avoiding the SET.
> 	(compute_costs): Likewise for the zero extension.  Use set_rtx_cost
> 	to compute the cost of moves.  Set the mode of the target register.
> 

FTR, this caused
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53278

R.
diff mbox

Patch

Index: gcc/lower-subreg.c
===================================================================
--- gcc/lower-subreg.c	2012-05-06 13:47:49.000000000 +0100
+++ gcc/lower-subreg.c	2012-05-06 14:56:47.851024108 +0100
@@ -135,13 +135,11 @@  struct cost_rtxes {
 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);
+  return set_src_cost (rtxes->shift, speed_p);
 }
 
 /* For each X in the range [0, BITS_PER_WORD), set SPLITTING[X]
@@ -189,11 +187,12 @@  compute_costs (bool speed_p, struct cost
   unsigned int i;
   int word_move_zero_cost, word_move_cost;
 
+  PUT_MODE (rtxes->target, word_mode);
   SET_SRC (rtxes->set) = CONST0_RTX (word_mode);
-  word_move_zero_cost = insn_rtx_cost (rtxes->set, speed_p);
+  word_move_zero_cost = set_rtx_cost (rtxes->set, speed_p);
 
   SET_SRC (rtxes->set) = rtxes->source;
-  word_move_cost = insn_rtx_cost (rtxes->set, speed_p);
+  word_move_cost = set_rtx_cost (rtxes->set, speed_p);
 
   if (LOG_COSTS)
     fprintf (stderr, "%s move: from zero cost %d, from reg cost %d\n",
@@ -209,7 +208,7 @@  compute_costs (bool speed_p, struct cost
 
 	  PUT_MODE (rtxes->target, mode);
 	  PUT_MODE (rtxes->source, mode);
-	  mode_move_cost = insn_rtx_cost (rtxes->set, speed_p);
+	  mode_move_cost = set_rtx_cost (rtxes->set, speed_p);
 
 	  if (LOG_COSTS)
 	    fprintf (stderr, "%s move: original cost %d, split cost %d * %d\n",
@@ -236,10 +235,8 @@  compute_costs (bool speed_p, struct cost
 
       /* The only case here to check to see if moving the upper part with a
 	 zero is cheaper than doing the zext itself.  */
-      PUT_MODE (rtxes->target, twice_word_mode);
       PUT_MODE (rtxes->source, word_mode);
-      SET_SRC (rtxes->set) = rtxes->zext;
-      zext_cost = insn_rtx_cost (rtxes->set, speed_p);
+      zext_cost = set_src_cost (rtxes->zext, speed_p);
 
       if (LOG_COSTS)
 	fprintf (stderr, "%s %s: original cost %d, split cost %d + %d\n",