Re: [committed] Fix lower-subreg cost calculation

2012-05-09 Thread Richard Earnshaw
On 08/05/12 22:42, Richard Sandiford wrote:
 Richard Earnshaw rearn...@arm.com writes:
 FTR, this caused
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53278
 
 Well, this really has been a brown-paper-bag patch.  Fixed as below.
 Tested on x86_64-linux-gnu and applied as obvious.
 
 Richard
 
 
 gcc/
   PR rtl-optimization/53278
   * lower-subreg.c (decompose_multiword_subregs): Remove left-over
   speed_p code from earlier patch.
 
 Index: gcc/lower-subreg.c
 ===
 --- gcc/lower-subreg.c2012-05-08 19:45:31.0 +0100
 +++ gcc/lower-subreg.c2012-05-08 19:45:31.793855523 +0100
 @@ -1487,9 +1487,7 @@ decompose_multiword_subregs (void)
FOR_EACH_BB (bb)
   {
 rtx insn;
 -   bool speed_p;
  
 -   speed_p = optimize_bb_for_speed_p (bb);
 FOR_BB_INSNS (bb, insn)
   {
 rtx pat;
 


Which begs the question as to why -wshadow didn't pick this up...

R.



Re: [committed] Fix lower-subreg cost calculation

2012-05-08 Thread Richard Sandiford
Richard Earnshaw rearn...@arm.com writes:
 FTR, this caused
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53278

Well, this really has been a brown-paper-bag patch.  Fixed as below.
Tested on x86_64-linux-gnu and applied as obvious.

Richard


gcc/
PR rtl-optimization/53278
* lower-subreg.c (decompose_multiword_subregs): Remove left-over
speed_p code from earlier patch.

Index: gcc/lower-subreg.c
===
--- gcc/lower-subreg.c  2012-05-08 19:45:31.0 +0100
+++ gcc/lower-subreg.c  2012-05-08 19:45:31.793855523 +0100
@@ -1487,9 +1487,7 @@ decompose_multiword_subregs (void)
   FOR_EACH_BB (bb)
{
  rtx insn;
- bool speed_p;
 
- speed_p = optimize_bb_for_speed_p (bb);
  FOR_BB_INSNS (bb, insn)
{
  rtx pat;


[committed] Fix lower-subreg cost calculation

2012-05-06 Thread Richard Sandiford
Georg-Johann Lay a...@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.

Index: gcc/lower-subreg.c
===
--- gcc/lower-subreg.c  2012-05-06 13:47:49.0 +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,