[PATCH 0/5] RFC, WIP: RTL cost improvements

[PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

This series creates pattern_cost and insn_cost functions that together
replace the existing insn_rtx_cost function.

pattern_cost is like the old insn_rtx_cost function; insn_cost takes
an actual rtx_insn * as input, not just a pattern.

Also a targetm.insn_cost is added, which targets can use to implement
a more exact cost more easily.

The combine patch is pretty gross (but functional), it needs some
refactoring (to not call recog so often).  The rs6000 patch is very
much a work in progress.

How does this look?  Is this the right direction?


Segher


Segher Boessenkool (5):
  Rename existing insn_cost to insn_sched_cost
  Replace insn_rtx_cost with insn_cost and pattern_cost
  combine: Use insn_cost instead of pattern_cost everywhere
  Add targetm.insn_cost hook
  rs6000: Implement insn_cost hook

 gcc/cfgrtl.c               |  7 +++----
 gcc/combine.c              | 40 +++++++++++++++++++++++++++++-----------
 gcc/config/rs6000/rs6000.c | 14 ++++++++++++++
 gcc/doc/tm.texi            | 12 ++++++++++++
 gcc/doc/tm.texi.in         |  2 ++
 gcc/dse.c                  |  2 +-
 gcc/haifa-sched.c          | 14 +++++++-------
 gcc/ifcvt.c                | 12 ++++++------
 gcc/rtl.h                  |  3 ++-
 gcc/rtlanal.c              | 16 ++++++++++++++--
 gcc/sched-int.h            |  2 +-
 gcc/sched-rgn.c            |  4 ++--
 gcc/sel-sched-ir.c         |  2 +-
 gcc/target.def             | 14 ++++++++++++++
 14 files changed, 108 insertions(+), 36 deletions(-)

-- 
1.9.3

[PATCH 2/5] Replace insn_rtx_cost with insn_cost and pattern_cost

[Segher Boessenkool]

This renames insn_rtx_cost to pattern cost, and adds a new function
insn_cost that takes an rtx_insn * instead of an instruction pattern
as input.  It uses the latter function anywhere an instruction is
readily available (instead of just an instruction pattern).

The actual implementation of insn_cost just calls pattern_cost on
the pattern of the instruction; no functional change yet.

---
 gcc/cfgrtl.c  |  7 +++----
 gcc/combine.c | 17 ++++++++---------
 gcc/dse.c     |  2 +-
 gcc/ifcvt.c   | 12 ++++++------
 gcc/rtl.h     |  3 ++-
 gcc/rtlanal.c | 13 +++++++++++--
 6 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 6ef47b7..739d1bb 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -5039,14 +5039,13 @@ rtl_account_profile_record (basic_block bb, int after_pass,
   FOR_BB_INSNS (bb, insn)
     if (INSN_P (insn))
       {
-	record->size[after_pass]
-	  += insn_rtx_cost (PATTERN (insn), false);
+	record->size[after_pass] += insn_cost (insn, false);
 	if (bb->count.initialized_p ())
 	  record->time[after_pass]
-	    += insn_rtx_cost (PATTERN (insn), true) * bb->count.to_gcov_type ();
+	    += insn_cost (insn, true) * bb->count.to_gcov_type ();
 	else if (profile_status_for_fn (cfun) == PROFILE_GUESSED)
 	  record->time[after_pass]
-	    += insn_rtx_cost (PATTERN (insn), true) * bb->frequency;
+	    += insn_cost (insn, true) * bb->frequency;
       }
 }
 
diff --git a/gcc/combine.c b/gcc/combine.c
index c5200db..aadd328 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -311,7 +311,7 @@ static bool optimize_this_for_speed_p;
 
 static int max_uid_known;
 
-/* The following array records the insn_rtx_cost for every insn
+/* The following array records the insn_cost for every insn
    in the instruction stream.  */
 
 static int *uid_insn_cost;
@@ -841,7 +841,7 @@ do_SUBST_LINK (struct insn_link **into, struct insn_link *newval)
 #define SUBST_LINK(oldval, newval) do_SUBST_LINK (&oldval, newval)
 \f
 /* Subroutine of try_combine.  Determine whether the replacement patterns
-   NEWPAT, NEWI2PAT and NEWOTHERPAT are cheaper according to insn_rtx_cost
+   NEWPAT, NEWI2PAT and NEWOTHERPAT are cheaper according to insn_cost
    than the original sequence I0, I1, I2, I3 and undobuf.other_insn.  Note
    that I0, I1 and/or NEWI2PAT may be NULL_RTX.  Similarly, NEWOTHERPAT and
    undobuf.other_insn may also both be NULL_RTX.  Return false if the cost
@@ -888,11 +888,11 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
     old_cost -= i1_cost;
 
 
-  /* Calculate the replacement insn_rtx_costs.  */
-  new_i3_cost = insn_rtx_cost (newpat, optimize_this_for_speed_p);
+  /* Calculate the replacement pattern_costs.  */
+  new_i3_cost = pattern_cost (newpat, optimize_this_for_speed_p);
   if (newi2pat)
     {
-      new_i2_cost = insn_rtx_cost (newi2pat, optimize_this_for_speed_p);
+      new_i2_cost = pattern_cost (newi2pat, optimize_this_for_speed_p);
       new_cost = (new_i2_cost > 0 && new_i3_cost > 0)
 		 ? new_i2_cost + new_i3_cost : 0;
     }
@@ -907,7 +907,7 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
       int old_other_cost, new_other_cost;
 
       old_other_cost = INSN_COST (undobuf.other_insn);
-      new_other_cost = insn_rtx_cost (newotherpat, optimize_this_for_speed_p);
+      new_other_cost = pattern_cost (newotherpat, optimize_this_for_speed_p);
       if (old_other_cost > 0 && new_other_cost > 0)
 	{
 	  old_cost += old_other_cost;
@@ -1208,10 +1208,9 @@ combine_instructions (rtx_insn *f, unsigned int nregs)
 		  set_nonzero_bits_and_sign_copies (XEXP (links, 0), NULL_RTX,
 						    insn);
 
-	    /* Record the current insn_rtx_cost of this instruction.  */
+	    /* Record the current insn_cost of this instruction.  */
 	    if (NONJUMP_INSN_P (insn))
-	      INSN_COST (insn) = insn_rtx_cost (PATTERN (insn),
-	      					optimize_this_for_speed_p);
+	      INSN_COST (insn) = insn_cost (insn, optimize_this_for_speed_p);
 	    if (dump_file)
 	      {
 		fprintf (dump_file, "insn_cost %d for ", INSN_COST (insn));
diff --git a/gcc/dse.c b/gcc/dse.c
index f87dd50..9ffb696 100644
--- a/gcc/dse.c
+++ b/gcc/dse.c
@@ -1650,7 +1650,7 @@ find_shift_sequence (int access_size,
       cost = 0;
       for (insn = shift_seq; insn != NULL_RTX; insn = NEXT_INSN (insn))
 	if (INSN_P (insn))
-	  cost += insn_rtx_cost (PATTERN (insn), speed);
+	  cost += insn_cost (insn, speed);
 
       /* The computation up to here is essentially independent
 	 of the arguments and could be precomputed.  It may
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 46a13c4..7de287f 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -121,7 +121,7 @@ count_bb_insns (const_basic_block bb)
   return count;
 }
 
-/* Determine whether the total insn_rtx_cost on non-jump insns in
+/* Determine whether the total insn_cost on non-jump insns in
    basic block BB is less than MAX_COST.  This function returns
    false if the cost of any instruction could not be estimated. 
 
@@ -140,7 +140,7 @@ cheap_bb_rtx_cost_p (const_basic_block bb,
 	      : REG_BR_PROB_BASE;
 
   /* Set scale to REG_BR_PROB_BASE to void the identical scaling
-     applied to insn_rtx_cost when optimizing for size.  Only do
+     applied to insn_cost when optimizing for size.  Only do
      this after combine because if-conversion might interfere with
      passes before combine.
 
@@ -163,7 +163,7 @@ cheap_bb_rtx_cost_p (const_basic_block bb,
     {
       if (NONJUMP_INSN_P (insn))
 	{
-	  int cost = insn_rtx_cost (PATTERN (insn), speed) * REG_BR_PROB_BASE;
+	  int cost = insn_cost (insn, speed) * REG_BR_PROB_BASE;
 	  if (cost == 0)
 	    return false;
 
@@ -3018,7 +3018,7 @@ bb_valid_for_noce_process_p (basic_block test_bb, rtx cond,
   if (first_insn == last_insn)
     {
       *simple_p = noce_operand_ok (SET_DEST (first_set));
-      *cost += insn_rtx_cost (first_set, speed_p);
+      *cost += pattern_cost (first_set, speed_p);
       return *simple_p;
     }
 
@@ -3034,7 +3034,7 @@ bb_valid_for_noce_process_p (basic_block test_bb, rtx cond,
   /* The regs that are live out of test_bb.  */
   bitmap test_bb_live_out = df_get_live_out (test_bb);
 
-  int potential_cost = insn_rtx_cost (last_set, speed_p);
+  int potential_cost = pattern_cost (last_set, speed_p);
   rtx_insn *insn;
   FOR_BB_INSNS (test_bb, insn)
     {
@@ -3054,7 +3054,7 @@ bb_valid_for_noce_process_p (basic_block test_bb, rtx cond,
 	      || reg_overlap_mentioned_p (SET_DEST (sset), cond))
 	    goto free_bitmap_and_fail;
 
-	  potential_cost += insn_rtx_cost (sset, speed_p);
+	  potential_cost += pattern_cost (sset, speed_p);
 	  bitmap_set_bit (test_bb_temps, REGNO (SET_DEST (sset)));
 	}
     }
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 8a68bb1..d2127b3 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3121,7 +3121,8 @@ extern int loc_mentioned_in_p (rtx *, const_rtx);
 extern rtx_insn *find_first_parameter_load (rtx_insn *, rtx_insn *);
 extern bool keep_with_call_p (const rtx_insn *);
 extern bool label_is_jump_target_p (const_rtx, const rtx_insn *);
-extern int insn_rtx_cost (rtx, bool);
+extern int pattern_cost (rtx, bool);
+extern int insn_cost (rtx_insn *, bool);
 extern unsigned seq_cost (const rtx_insn *, bool);
 
 /* Given an insn and condition, return a canonical description of
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 9bfae8c..8f05546 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5254,11 +5254,11 @@ num_sign_bit_copies1 (const_rtx x, machine_mode mode, const_rtx known_x,
 	 ? 1 : bitwidth - floor_log2 (nonzero) - 1;
 }
 
-/* Calculate the rtx_cost of a single instruction.  A return value of
+/* Calculate the rtx_cost of a single instruction pattern.  A return value of
    zero indicates an instruction pattern without a known cost.  */
 
 int
-insn_rtx_cost (rtx pat, bool speed)
+pattern_cost (rtx pat, bool speed)
 {
   int i, cost;
   rtx set;
@@ -5308,6 +5308,15 @@ insn_rtx_cost (rtx pat, bool speed)
   return cost > 0 ? cost : COSTS_N_INSNS (1);
 }
 
+/* Calculate the cost of a single instruction.  A return value of zero
+   indicates an instruction pattern without a known cost.  */
+
+int
+insn_cost (rtx_insn *insn, bool speed)
+{
+  return pattern_cost (PATTERN (insn), speed);
+}
+
 /* Returns estimate on cost of computing SEQ.  */
 
 unsigned
-- 
1.9.3

Re: [PATCH 2/5] Replace insn_rtx_cost with insn_cost and pattern_cost

[Jeff Law]

On 07/31/2017 05:25 PM, Segher Boessenkool wrote:
> This renames insn_rtx_cost to pattern cost, and adds a new function
> insn_cost that takes an rtx_insn * instead of an instruction pattern
> as input.  It uses the latter function anywhere an instruction is
> readily available (instead of just an instruction pattern).
> 
> The actual implementation of insn_cost just calls pattern_cost on
> the pattern of the instruction; no functional change yet.
> 
> ---
>  gcc/cfgrtl.c  |  7 +++----
>  gcc/combine.c | 17 ++++++++---------
>  gcc/dse.c     |  2 +-
>  gcc/ifcvt.c   | 12 ++++++------
>  gcc/rtl.h     |  3 ++-
>  gcc/rtlanal.c | 13 +++++++++++--
>  6 files changed, 31 insertions(+), 23 deletions(-)
Also looks good to me to go in once you've got a ChangeLog.

jeff

[PATCH 3/5] combine: Use insn_cost instead of pattern_cost everywhere

[Segher Boessenkool]

This changes combine to always use insn_cost, not pattern_cost.  I don't
intend to commit it like this: it calls recog more often than necessary,
and it is very ugly (no, don't look at it).  But it's good enough to test
things with.

---
 gcc/combine.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index aadd328..d0bd4d3 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -856,7 +856,7 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
   int new_i2_cost, new_i3_cost;
   int old_cost, new_cost;
 
-  /* Lookup the original insn_rtx_costs.  */
+  /* Lookup the original insn_costs.  */
   i2_cost = INSN_COST (i2);
   i3_cost = INSN_COST (i3);
 
@@ -888,11 +888,23 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
     old_cost -= i1_cost;
 
 
-  /* Calculate the replacement pattern_costs.  */
-  new_i3_cost = pattern_cost (newpat, optimize_this_for_speed_p);
+  /* Calculate the replacement insn_costs.  */
+  rtx tmp = PATTERN (i3);
+  PATTERN (i3) = newpat;
+  int tmpi = INSN_CODE (i3);
+  INSN_CODE (i3) = -1;
+  new_i3_cost = insn_cost (i3, optimize_this_for_speed_p);
+  PATTERN (i3) = tmp;
+  INSN_CODE (i3) = tmpi;
   if (newi2pat)
     {
-      new_i2_cost = pattern_cost (newi2pat, optimize_this_for_speed_p);
+      tmp = PATTERN (i2);
+      PATTERN (i2) = newi2pat;
+      tmpi = INSN_CODE (i2);
+      INSN_CODE (i2) = -1;
+      new_i2_cost = insn_cost (i2, optimize_this_for_speed_p);
+      PATTERN (i2) = tmp;
+      INSN_CODE (i2) = tmpi;
       new_cost = (new_i2_cost > 0 && new_i3_cost > 0)
 		 ? new_i2_cost + new_i3_cost : 0;
     }
@@ -907,7 +919,14 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
       int old_other_cost, new_other_cost;
 
       old_other_cost = INSN_COST (undobuf.other_insn);
-      new_other_cost = pattern_cost (newotherpat, optimize_this_for_speed_p);
+      tmp = PATTERN (undobuf.other_insn);
+      PATTERN (undobuf.other_insn) = newotherpat;
+      tmpi = INSN_CODE (undobuf.other_insn);
+      INSN_CODE (undobuf.other_insn) = -1;
+      new_other_cost = insn_cost (undobuf.other_insn,
+				  optimize_this_for_speed_p);
+      PATTERN (undobuf.other_insn) = tmp;
+      INSN_CODE (undobuf.other_insn) = tmpi;
       if (old_other_cost > 0 && new_other_cost > 0)
 	{
 	  old_cost += old_other_cost;
@@ -4081,7 +4100,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 	}
     }
 
-  /* Only allow this combination if insn_rtx_costs reports that the
+  /* Only allow this combination if insn_cost reports that the
      replacement instructions are cheaper than the originals.  */
   if (!combine_validate_cost (i0, i1, i2, i3, newpat, newi2pat, other_pat))
     {
-- 
1.9.3

Re: [PATCH 3/5] combine: Use insn_cost instead of pattern_cost everywhere

[Jeff Law]

On 07/31/2017 05:25 PM, Segher Boessenkool wrote:
> This changes combine to always use insn_cost, not pattern_cost.  I don't
> intend to commit it like this: it calls recog more often than necessary,
> and it is very ugly (no, don't look at it).  But it's good enough to test
> things with.
So what do you expect this is going to look like when you're done?  The
other target independent patches all look reasonable so I think it's
really a matter of how you want to want to use the new infrastructure in
combine.c (which will become the template for how other passes might use
the infrastructure as well).

jeff

Re: [PATCH 3/5] combine: Use insn_cost instead of pattern_cost everywhere

[Segher Boessenkool]

On Wed, Aug 02, 2017 at 10:40:32AM -0600, Jeff Law wrote:
> On 07/31/2017 05:25 PM, Segher Boessenkool wrote:
> > This changes combine to always use insn_cost, not pattern_cost.  I don't
> > intend to commit it like this: it calls recog more often than necessary,
> > and it is very ugly (no, don't look at it).  But it's good enough to test
> > things with.
> So what do you expect this is going to look like when you're done?  The
> other target independent patches all look reasonable so I think it's
> really a matter of how you want to want to use the new infrastructure in
> combine.c (which will become the template for how other passes might use
> the infrastructure as well).

Just like this: use insn_cost everywhere instead of pattern_cost.
The change still needed is that in the current prototype here I have
to swap in and out the new patterns more often than necessary, and
call recog twice for every new pattern (the first time to see if
combine came up with insns that exist at all, the second time to see
how expensive they are).  This of course is a bit wasteful (recog isn't
cheap), and unnecessary.

Patch 2 already uses insn_cost (instead of pattern_cost / insn_rtx_cost)
in cfgrtl.c, dse.c, and in ifcvt.c in one place (the other three places
weren't trivial to convert).


Segher

[PATCH 4/5] Add targetm.insn_cost hook

[Segher Boessenkool]

This introduces a hook to implement insn_cost.  If a target does not
implement the hook, the old function (i.e. pattern_cost) is used.

---
 gcc/doc/tm.texi    | 12 ++++++++++++
 gcc/doc/tm.texi.in |  2 ++
 gcc/rtlanal.c      |  3 +++
 gcc/target.def     | 14 ++++++++++++++
 4 files changed, 31 insertions(+)

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 23e85c7..dfba37f 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6640,6 +6640,18 @@ should probably only be given to addresses with different numbers of
 registers on machines with lots of registers.
 @end deftypefn
 
+@deftypefn {Target Hook} int TARGET_INSN_COST (rtx_insn *@var{insn}, bool @var{speed})
+This target hook describes the relative costs of RTL instructions.
+
+In implementing this hook, you can use the construct
+@code{COSTS_N_INSNS (@var{n})} to specify a cost equal to @var{n} fast
+instructions.
+
+When optimizing for code size, i.e.@: when @code{speed} is
+false, this target hook should be used to estimate the relative
+size cost of an expression, again relative to @code{COSTS_N_INSNS}.
+@end deftypefn
+
 @deftypefn {Target Hook} {unsigned int} TARGET_MAX_NOCE_IFCVT_SEQ_COST (edge @var{e})
 This hook returns a value in the same units as @code{TARGET_RTX_COSTS},
 giving the maximum acceptable cost for a sequence generated by the RTL
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 6df08a2..6e81341 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4796,6 +4796,8 @@ Define this macro if a non-short-circuit operation produced by
 
 @hook TARGET_ADDRESS_COST
 
+@hook TARGET_INSN_COST
+
 @hook TARGET_MAX_NOCE_IFCVT_SEQ_COST
 
 @hook TARGET_NOCE_CONVERSION_PROFITABLE_P
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 8f05546..aca194e 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5314,6 +5314,9 @@ pattern_cost (rtx pat, bool speed)
 int
 insn_cost (rtx_insn *insn, bool speed)
 {
+  if (targetm.insn_cost)
+    return targetm.insn_cost (insn, speed);
+
   return pattern_cost (PATTERN (insn), speed);
 }
 
diff --git a/gcc/target.def b/gcc/target.def
index 6d67b1f..2eb411d 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -3647,6 +3647,20 @@ registers on machines with lots of registers.",
  int, (rtx address, machine_mode mode, addr_space_t as, bool speed),
  default_address_cost)
 
+/* Compute a cost for INSN.  */
+DEFHOOK
+(insn_cost,
+ "This target hook describes the relative costs of RTL instructions.\n\
+\n\
+In implementing this hook, you can use the construct\n\
+@code{COSTS_N_INSNS (@var{n})} to specify a cost equal to @var{n} fast\n\
+instructions.\n\
+\n\
+When optimizing for code size, i.e.@: when @code{speed} is\n\
+false, this target hook should be used to estimate the relative\n\
+size cost of an expression, again relative to @code{COSTS_N_INSNS}.",
+ int, (rtx_insn *insn, bool speed), NULL)
+
 /* Give a cost, in RTX Costs units, for an edge.  Like BRANCH_COST, but with
    well defined units.  */
 DEFHOOK
-- 
1.9.3

Re: [PATCH 4/5] Add targetm.insn_cost hook

[Jeff Law]

On 07/31/2017 05:25 PM, Segher Boessenkool wrote:
> This introduces a hook to implement insn_cost.  If a target does not
> implement the hook, the old function (i.e. pattern_cost) is used.
> 
> ---
>  gcc/doc/tm.texi    | 12 ++++++++++++
>  gcc/doc/tm.texi.in |  2 ++
>  gcc/rtlanal.c      |  3 +++
>  gcc/target.def     | 14 ++++++++++++++
>  4 files changed, 31 insertions(+)
Looks reasonable as well.

jeff

[PATCH 1/5] Rename existing insn_cost to insn_sched_cost

[Segher Boessenkool]

haifa-sched exports an insn_cost function, but it is only used in a
few places and specialised to scheduling.  This patch renames it to
insn_sched_cost.

---
 gcc/haifa-sched.c  | 14 +++++++-------
 gcc/sched-int.h    |  2 +-
 gcc/sched-rgn.c    |  4 ++--
 gcc/sel-sched-ir.c |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index af0ed27..d1378d0 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -225,7 +225,7 @@ struct common_sched_info_def *common_sched_info;
 #define FEEDS_BACKTRACK_INSN(INSN) (HID (INSN)->feeds_backtrack_insn)
 #define SHADOW_P(INSN) (HID (INSN)->shadow_p)
 #define MUST_RECOMPUTE_SPEC_P(INSN) (HID (INSN)->must_recompute_spec)
-/* Cached cost of the instruction.  Use insn_cost to get cost of the
+/* Cached cost of the instruction.  Use insn_sched_cost to get cost of the
    insn.  -1 here means that the field is not initialized.  */
 #define INSN_COST(INSN)	(HID (INSN)->cost)
 
@@ -1383,7 +1383,7 @@ static rtx_insn *nonscheduled_insns_begin;
    This is the number of cycles between instruction issue and
    instruction results.  */
 int
-insn_cost (rtx_insn *insn)
+insn_sched_cost (rtx_insn *insn)
 {
   int cost;
 
@@ -1470,7 +1470,7 @@ dep_cost_1 (dep_t link, dw_t dw)
     {
       enum reg_note dep_type = DEP_TYPE (link);
 
-      cost = insn_cost (insn);
+      cost = insn_sched_cost (insn);
 
       if (INSN_CODE (insn) >= 0)
 	{
@@ -1608,11 +1608,11 @@ priority (rtx_insn *insn)
 	  INSN_FUSION_PRIORITY (insn) = this_fusion_priority;
 	}
       else if (dep_list_size (insn, SD_LIST_FORW) == 0)
-	/* ??? We should set INSN_PRIORITY to insn_cost when and insn has
-	   some forward deps but all of them are ignored by
+	/* ??? We should set INSN_PRIORITY to insn_sched_cost when and insn
+	   has some forward deps but all of them are ignored by
 	   contributes_to_priority hook.  At the moment we set priority of
 	   such insn to 0.  */
-	this_priority = insn_cost (insn);
+	this_priority = insn_sched_cost (insn);
       else
 	{
 	  rtx_insn *prev_first, *twin;
@@ -1683,7 +1683,7 @@ priority (rtx_insn *insn)
 	{
 	  gcc_assert (this_priority == -1);
 
-	  this_priority = insn_cost (insn);
+	  this_priority = insn_sched_cost (insn);
 	}
 
       INSN_PRIORITY (insn) = this_priority;
diff --git a/gcc/sched-int.h b/gcc/sched-int.h
index 624d892..2af8f9f 100644
--- a/gcc/sched-int.h
+++ b/gcc/sched-int.h
@@ -1403,7 +1403,7 @@ extern void get_ebb_head_tail (basic_block, basic_block,
 			       rtx_insn **, rtx_insn **);
 extern int no_real_insns_p (const rtx_insn *, const rtx_insn *);
 
-extern int insn_cost (rtx_insn *);
+extern int insn_sched_cost (rtx_insn *);
 extern int dep_cost_1 (dep_t, dw_t);
 extern int dep_cost (dep_t);
 extern int set_priorities (rtx_insn *, rtx_insn *);
diff --git a/gcc/sched-rgn.c b/gcc/sched-rgn.c
index 492094e..ba5b47c 100644
--- a/gcc/sched-rgn.c
+++ b/gcc/sched-rgn.c
@@ -2837,8 +2837,8 @@ void debug_dependencies (rtx_insn *head, rtx_insn *tail)
 			       : INSN_PRIORITY (insn))
 		: INSN_PRIORITY (insn)),
 	       (sel_sched_p () ? (sched_emulate_haifa_p ? -1
-			       : insn_cost (insn))
-		: insn_cost (insn)));
+			       : insn_sched_cost (insn))
+		: insn_sched_cost (insn)));
 
       if (recog_memoized (insn) < 0)
 	fprintf (sched_dump, "nothing");
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index fa88259..c0e835f 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -1324,7 +1324,7 @@ sel_insn_rtx_cost (rtx_insn *insn)
 }
 
 /* Return the cost of the VI.
-   !!! FIXME: Unify with haifa-sched.c: insn_cost ().  */
+   !!! FIXME: Unify with haifa-sched.c: insn_sched_cost ().  */
 int
 sel_vinsn_cost (vinsn_t vi)
 {
-- 
1.9.3

Re: [PATCH 1/5] Rename existing insn_cost to insn_sched_cost

[Jeff Law]

On 07/31/2017 05:25 PM, Segher Boessenkool wrote:
> haifa-sched exports an insn_cost function, but it is only used in a
> few places and specialised to scheduling.  This patch renames it to
> insn_sched_cost.
> 
> ---
>  gcc/haifa-sched.c  | 14 +++++++-------
>  gcc/sched-int.h    |  2 +-
>  gcc/sched-rgn.c    |  4 ++--
>  gcc/sel-sched-ir.c |  2 +-
>  4 files changed, 11 insertions(+), 11 deletions(-)
This seems fine independent of the following patches.  So with a
ChangeLog it should probably go in now rather than later.

jeff

[PATCH 5/5] rs6000: Implement insn_cost hook

[Segher Boessenkool]

This is a pretty minimalistic implementation of the insn_cost hook: it
just counts how many machine instructions will be generated.  Some
improvements are needed: loads should get extra cost; some instructions
like mul and div should be more expensive than others; and it exposes
some suboptimalities in our machine description files.  Still, good
enough for most testing.

---
 gcc/config/rs6000/rs6000.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index a7d2e7e..b4fda69 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1761,6 +1761,8 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #define TARGET_RTX_COSTS rs6000_rtx_costs
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST hook_int_rtx_mode_as_bool_0
+#undef TARGET_INSN_COST
+#define TARGET_INSN_COST rs6000_insn_cost
 
 #undef TARGET_INIT_DWARF_REG_SIZES_EXTRA
 #define TARGET_INIT_DWARF_REG_SIZES_EXTRA rs6000_init_dwarf_reg_sizes_extra
@@ -34521,6 +34523,18 @@ rs6000_debug_rtx_costs (rtx x, machine_mode mode, int outer_code,
   return ret;
 }
 
+static int
+rs6000_insn_cost (rtx_insn *insn, bool speed)
+{
+  if (recog_memoized (insn) < 0)
+    return 0;
+
+  if (!speed)
+    return get_attr_length (insn);
+
+  return COSTS_N_INSNS (get_attr_length (insn) / 4);
+}
+
 /* Debug form of ADDRESS_COST that is selected if -mdebug=cost.  */
 
 static int
-- 
1.9.3

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Biener]

On Tue, Aug 1, 2017 at 1:24 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> This series creates pattern_cost and insn_cost functions that together
> replace the existing insn_rtx_cost function.
>
> pattern_cost is like the old insn_rtx_cost function; insn_cost takes
> an actual rtx_insn * as input, not just a pattern.
>
> Also a targetm.insn_cost is added, which targets can use to implement
> a more exact cost more easily.
>
> The combine patch is pretty gross (but functional), it needs some
> refactoring (to not call recog so often).  The rs6000 patch is very
> much a work in progress.
>
> How does this look?  Is this the right direction?

I think it is the right direction.  I'll leave the details to somebody more
familiar with RTL.

Richard.

>
> Segher
>
>
> Segher Boessenkool (5):
>   Rename existing insn_cost to insn_sched_cost
>   Replace insn_rtx_cost with insn_cost and pattern_cost
>   combine: Use insn_cost instead of pattern_cost everywhere
>   Add targetm.insn_cost hook
>   rs6000: Implement insn_cost hook
>
>  gcc/cfgrtl.c               |  7 +++----
>  gcc/combine.c              | 40 +++++++++++++++++++++++++++++-----------
>  gcc/config/rs6000/rs6000.c | 14 ++++++++++++++
>  gcc/doc/tm.texi            | 12 ++++++++++++
>  gcc/doc/tm.texi.in         |  2 ++
>  gcc/dse.c                  |  2 +-
>  gcc/haifa-sched.c          | 14 +++++++-------
>  gcc/ifcvt.c                | 12 ++++++------
>  gcc/rtl.h                  |  3 ++-
>  gcc/rtlanal.c              | 16 ++++++++++++++--
>  gcc/sched-int.h            |  2 +-
>  gcc/sched-rgn.c            |  4 ++--
>  gcc/sel-sched-ir.c         |  2 +-
>  gcc/target.def             | 14 ++++++++++++++
>  14 files changed, 108 insertions(+), 36 deletions(-)
>
> --
> 1.9.3
>

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> This series creates pattern_cost and insn_cost functions that together
> replace the existing insn_rtx_cost function.
>
> pattern_cost is like the old insn_rtx_cost function; insn_cost takes
> an actual rtx_insn * as input, not just a pattern.
>
> Also a targetm.insn_cost is added, which targets can use to implement
> a more exact cost more easily.
>
> The combine patch is pretty gross (but functional), it needs some
> refactoring (to not call recog so often).  The rs6000 patch is very
> much a work in progress.
>
> How does this look?  Is this the right direction?

Seems good to me FWIW.  Since this hook is entirely new, would it
be worth standardising on attribute names for size and speed costs,
a bit like "length" and "enabled"?  I think otherwise the target hooks
are going to end up with similar boilerplate.

Thanks,
Richard

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Sat, Aug 05, 2017 at 11:07:20AM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > This series creates pattern_cost and insn_cost functions that together
> > replace the existing insn_rtx_cost function.
> >
> > pattern_cost is like the old insn_rtx_cost function; insn_cost takes
> > an actual rtx_insn * as input, not just a pattern.
> >
> > Also a targetm.insn_cost is added, which targets can use to implement
> > a more exact cost more easily.
> >
> > The combine patch is pretty gross (but functional), it needs some
> > refactoring (to not call recog so often).  The rs6000 patch is very
> > much a work in progress.
> >
> > How does this look?  Is this the right direction?
> 
> Seems good to me FWIW.  Since this hook is entirely new, would it
> be worth standardising on attribute names for size and speed costs,
> a bit like "length" and "enabled"?  I think otherwise the target hooks
> are going to end up with similar boilerplate.

For size cost I currently use just "length", but I haven't looked at
size cost much at all yet.

For speed cost I primarily use "type", modified by the number of machine
insns a pattern generates (quite a few are split); and I get the number
of machine insns just from "length" again, which for rs6000 is easy and
correct in most cases.  Some other targets may need something else.

I also have an attribute "cost" that can be used to override the
default calculation; that seems useful to standardise on.  I've pondered
a "cost_adjust" that will be added to the calculated cost instead, but
it hasn't been useful so far.

There are two types of insns that happen in the insn stream but which
aren't recognised: asm, and no-op moves.  I cost both as cost 0 currently.

"length" and "enabled" are special, they are used by non-target code.
So far I don't think it is useful to handle "cost" specially, but it
is such a nice short name that I expect many targets will want it,
whether it is handled generically or not :-)


Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Jeff Law]

On 08/05/2017 11:15 AM, Segher Boessenkool wrote:
> On Sat, Aug 05, 2017 at 11:07:20AM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>> This series creates pattern_cost and insn_cost functions that together
>>> replace the existing insn_rtx_cost function.
>>>
>>> pattern_cost is like the old insn_rtx_cost function; insn_cost takes
>>> an actual rtx_insn * as input, not just a pattern.
>>>
>>> Also a targetm.insn_cost is added, which targets can use to implement
>>> a more exact cost more easily.
>>>
>>> The combine patch is pretty gross (but functional), it needs some
>>> refactoring (to not call recog so often).  The rs6000 patch is very
>>> much a work in progress.
>>>
>>> How does this look?  Is this the right direction?
>>
>> Seems good to me FWIW.  Since this hook is entirely new, would it
>> be worth standardising on attribute names for size and speed costs,
>> a bit like "length" and "enabled"?  I think otherwise the target hooks
>> are going to end up with similar boilerplate.
> 
> For size cost I currently use just "length", but I haven't looked at
> size cost much at all yet.
I think that's fine.  "length" is pretty standardized at this point and
it's the right metric.  For ports that don't bother defining a length
attribute, punt in some reasonable manner.



> 
> For speed cost I primarily use "type", modified by the number of machine
> insns a pattern generates (quite a few are split); and I get the number
> of machine insns just from "length" again, which for rs6000 is easy and
> correct in most cases.  Some other targets may need something else.

> 
> I also have an attribute "cost" that can be used to override the
> default calculation; that seems useful to standardise on.  I've pondered
> a "cost_adjust" that will be added to the calculated cost instead, but
> it hasn't been useful so far.
Let's go ahead and "reserve" cost and cost_adjust for this purpose.  If
we find that cost_adjust isn't actually necessary, then so be it, it's
not a big deal to me.

Jeff

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Jeff Law <law@redhat.com> writes:
> On 08/05/2017 11:15 AM, Segher Boessenkool wrote:
>> On Sat, Aug 05, 2017 at 11:07:20AM +0100, Richard Sandiford wrote:
>>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>>> This series creates pattern_cost and insn_cost functions that together
>>>> replace the existing insn_rtx_cost function.
>>>>
>>>> pattern_cost is like the old insn_rtx_cost function; insn_cost takes
>>>> an actual rtx_insn * as input, not just a pattern.
>>>>
>>>> Also a targetm.insn_cost is added, which targets can use to implement
>>>> a more exact cost more easily.
>>>>
>>>> The combine patch is pretty gross (but functional), it needs some
>>>> refactoring (to not call recog so often).  The rs6000 patch is very
>>>> much a work in progress.
>>>>
>>>> How does this look?  Is this the right direction?
>>>
>>> Seems good to me FWIW.  Since this hook is entirely new, would it
>>> be worth standardising on attribute names for size and speed costs,
>>> a bit like "length" and "enabled"?  I think otherwise the target hooks
>>> are going to end up with similar boilerplate.
>> 
>> For size cost I currently use just "length", but I haven't looked at
>> size cost much at all yet.
> I think that's fine.  "length" is pretty standardized at this point and
> it's the right metric.  For ports that don't bother defining a length
> attribute, punt in some reasonable manner.
>
>
>
>> 
>> For speed cost I primarily use "type", modified by the number of machine
>> insns a pattern generates (quite a few are split); and I get the number
>> of machine insns just from "length" again, which for rs6000 is easy and
>> correct in most cases.  Some other targets may need something else.
>
>> 
>> I also have an attribute "cost" that can be used to override the
>> default calculation; that seems useful to standardise on.  I've pondered
>> a "cost_adjust" that will be added to the calculated cost instead, but
>> it hasn't been useful so far.
> Let's go ahead and "reserve" cost and cost_adjust for this purpose.  If
> we find that cost_adjust isn't actually necessary, then so be it, it's
> not a big deal to me.

I was thinking we should have separate attributes for size and speed
from the outset.   How about size_cost and speed_cost?  It'd be up
to the target to decide whether to define them as the sums of various
sub-attributes (like it's the target's decision how to break "enabled"
down).

The advantage of doing it all in attributes is that the generator might
be able to help optimise the process of checking for costs.  No promises
though :-)

TBH I think we should start with the attributes as well-defined names
and only add the hook if we find we still need it.  I can't really see
what a C function would give over keeping the costs with the instruction
definitions.

Thanks,
Richard

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Jeff Law]

On 08/08/2017 10:54 AM, Richard Sandiford wrote:
>>> For speed cost I primarily use "type", modified by the number of machine
>>> insns a pattern generates (quite a few are split); and I get the number
>>> of machine insns just from "length" again, which for rs6000 is easy and
>>> correct in most cases.  Some other targets may need something else.
>>
>>>
>>> I also have an attribute "cost" that can be used to override the
>>> default calculation; that seems useful to standardise on.  I've pondered
>>> a "cost_adjust" that will be added to the calculated cost instead, but
>>> it hasn't been useful so far.
>> Let's go ahead and "reserve" cost and cost_adjust for this purpose.  If
>> we find that cost_adjust isn't actually necessary, then so be it, it's
>> not a big deal to me.
> 
> I was thinking we should have separate attributes for size and speed
> from the outset.   How about size_cost and speed_cost?  It'd be up
> to the target to decide whether to define them as the sums of various
> sub-attributes (like it's the target's decision how to break "enabled"
> down).
But size_cost should be equivalent to the already standardized length
attribute.  So I'm struggling to see a need for that.

Again, no strong opinions on how to structure the speed cost other than
to standardize a name.


> 
> The advantage of doing it all in attributes is that the generator might
> be able to help optimise the process of checking for costs.  No promises
> though :-)
 :-)

> 
> TBH I think we should start with the attributes as well-defined names
> and only add the hook if we find we still need it.  I can't really see
> what a C function would give over keeping the costs with the instruction
> definitions.
I'd think the C function would mostly be helpful on a cisc target where
operands are potentially complex.   But I don't feel strongly enough
about it to argue -- I'm happy to go with consensus and fault in
adjustments if we need them.

jeff

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Jeff Law <law@redhat.com> writes:
> On 08/08/2017 10:54 AM, Richard Sandiford wrote:
>>>> For speed cost I primarily use "type", modified by the number of machine
>>>> insns a pattern generates (quite a few are split); and I get the number
>>>> of machine insns just from "length" again, which for rs6000 is easy and
>>>> correct in most cases.  Some other targets may need something else.
>>>
>>>>
>>>> I also have an attribute "cost" that can be used to override the
>>>> default calculation; that seems useful to standardise on.  I've pondered
>>>> a "cost_adjust" that will be added to the calculated cost instead, but
>>>> it hasn't been useful so far.
>>> Let's go ahead and "reserve" cost and cost_adjust for this purpose.  If
>>> we find that cost_adjust isn't actually necessary, then so be it, it's
>>> not a big deal to me.
>> 
>> I was thinking we should have separate attributes for size and speed
>> from the outset.   How about size_cost and speed_cost?  It'd be up
>> to the target to decide whether to define them as the sums of various
>> sub-attributes (like it's the target's decision how to break "enabled"
>> down).
> But size_cost should be equivalent to the already standardized length
> attribute.  So I'm struggling to see a need for that.

COSTS_N_INSNS lets you add sub-instruction costs, so that you can
slightly prefer faster N-byte instructions to slower N-byte instructions.

> Again, no strong opinions on how to structure the speed cost other than
> to standardize a name.
>
>
>> 
>> The advantage of doing it all in attributes is that the generator might
>> be able to help optimise the process of checking for costs.  No promises
>> though :-)
>  :-)
>
>> 
>> TBH I think we should start with the attributes as well-defined names
>> and only add the hook if we find we still need it.  I can't really see
>> what a C function would give over keeping the costs with the instruction
>> definitions.
> I'd think the C function would mostly be helpful on a cisc target where
> operands are potentially complex.   But I don't feel strongly enough
> about it to argue -- I'm happy to go with consensus and fault in
> adjustments if we need them.

I think even the complex operands can be handled by attributes: you can
have one enum attribute that describes the complexity, perhaps with the
default value calling out to a C function if necessary, and then make
the default value of the cost attribute switch on that enum attribute.

Thanks,
Richard

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Wed, Aug 09, 2017 at 09:56:31AM -0600, Jeff Law wrote:
> On 08/08/2017 10:54 AM, Richard Sandiford wrote:
> > I was thinking we should have separate attributes for size and speed
> > from the outset.   How about size_cost and speed_cost?  It'd be up
> > to the target to decide whether to define them as the sums of various
> > sub-attributes (like it's the target's decision how to break "enabled"
> > down).
> But size_cost should be equivalent to the already standardized length
> attribute.  So I'm struggling to see a need for that.

"length" is (by necessity) pessimistic, cost doesn't have to be.  Not
that it will make much difference as far as I can see.

> > The advantage of doing it all in attributes is that the generator might
> > be able to help optimise the process of checking for costs.  No promises
> > though :-)
>  :-)

I was thinking I could have "cost" (for rs6000) have a default value
that looks at "type".  This can then optimise things a little bit.
But, we probably still need the "function" version as well, for the more
complex cases.  Simpler targets can do this of course.

> > TBH I think we should start with the attributes as well-defined names
> > and only add the hook if we find we still need it.

We need it, for example, to properly cost the various define_insn_and_split
(for which "type" is only an approximation, and is woefully inadequate
for determining cost).

> > I can't really see
> > what a C function would give over keeping the costs with the instruction
> > definitions.

There are many insn patterns that can share the cost computation.

> I'd think the C function would mostly be helpful on a cisc target where
> operands are potentially complex.   But I don't feel strongly enough
> about it to argue -- I'm happy to go with consensus and fault in
> adjustments if we need them.

Making this a hook is by far the most flexible.

One place where operands are nasty on rs6000 is subregs of float.  Those
are used all over the place, and they aren't turned into (expensive!)
insns until LRA.  In the insn patterns they look just like a SI (or DI)
reg :-(

Another example is unaligned MEMs, which can be very expensive, much
more expensive than aligned MEMs.

The common theme is that we care most about cost in the not-so-happy
cases, and those are often not easy to express in attributes.


Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Wed, Aug 09, 2017 at 09:56:31AM -0600, Jeff Law wrote:
>> On 08/08/2017 10:54 AM, Richard Sandiford wrote:
>> > I was thinking we should have separate attributes for size and speed
>> > from the outset.   How about size_cost and speed_cost?  It'd be up
>> > to the target to decide whether to define them as the sums of various
>> > sub-attributes (like it's the target's decision how to break "enabled"
>> > down).
>> But size_cost should be equivalent to the already standardized length
>> attribute.  So I'm struggling to see a need for that.
>
> "length" is (by necessity) pessimistic, cost doesn't have to be.  Not
> that it will make much difference as far as I can see.
>
>> > The advantage of doing it all in attributes is that the generator might
>> > be able to help optimise the process of checking for costs.  No promises
>> > though :-)
>>  :-)
>
> I was thinking I could have "cost" (for rs6000) have a default value
> that looks at "type".  This can then optimise things a little bit.
> But, we probably still need the "function" version as well, for the more
> complex cases.  Simpler targets can do this of course.
>
>> > TBH I think we should start with the attributes as well-defined names
>> > and only add the hook if we find we still need it.
>
> We need it, for example, to properly cost the various define_insn_and_split
> (for which "type" is only an approximation, and is woefully inadequate
> for determining cost).

But define_insn_and_splits could override the cost explicitly if they
need to.  That seems neater than testing for them in C.

>> > I can't really see
>> > what a C function would give over keeping the costs with the instruction
>> > definitions.
>
> There are many insn patterns that can share the cost computation.
>
>> I'd think the C function would mostly be helpful on a cisc target where
>> operands are potentially complex.   But I don't feel strongly enough
>> about it to argue -- I'm happy to go with consensus and fault in
>> adjustments if we need them.
>
> Making this a hook is by far the most flexible.

Maybe, but if so, that would probably also have been true for "enabled"
and "length".  We gained something by not making them hooks.

That said...

> One place where operands are nasty on rs6000 is subregs of float.  Those
> are used all over the place, and they aren't turned into (expensive!)
> insns until LRA.  In the insn patterns they look just like a SI (or DI)
> reg :-(
>
> Another example is unaligned MEMs, which can be very expensive, much
> more expensive than aligned MEMs.

...thanks, I agree these are convincing.

Richard

>
> The common theme is that we care most about cost in the not-so-happy
> cases, and those are often not easy to express in attributes.
>
>
> Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Wed, Aug 09, 2017 at 05:54:40PM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > We need it, for example, to properly cost the various define_insn_and_split
> > (for which "type" is only an approximation, and is woefully inadequate
> > for determining cost).
> 
> But define_insn_and_splits could override the cost explicitly if they
> need to.  That seems neater than testing for them in C.

All 190 of them?  Not counting those that are define_insn+define_split
(we still have way too many of those).

Neat, indeed, but not altogether practical :-(


Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Wed, Aug 09, 2017 at 05:54:40PM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > We need it, for example, to properly cost the various define_insn_and_split
> > (for which "type" is only an approximation, and is woefully inadequate
> > for determining cost).
> 
> But define_insn_and_splits could override the cost explicitly if they
> need to.  That seems neater than testing for them in C.

All 190 of them?  Not counting those that are define_insn+define_split
(we still have way too many of those).

Neat, indeed, but not altogether practical :-(


Segher

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Wed, Aug 09, 2017 at 05:54:40PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> > We need it, for example, to properly cost the various define_insn_and_split
>> > (for which "type" is only an approximation, and is woefully inadequate
>> > for determining cost).
>> 
>> But define_insn_and_splits could override the cost explicitly if they
>> need to.  That seems neater than testing for them in C.
>
> All 190 of them?  Not counting those that are define_insn+define_split
> (we still have way too many of those).
>
> Neat, indeed, but not altogether practical :-(

Are there really 190 separate cases though?  If not, then it's
possible to have separate attributes that describe various forms of
multi-instruction pattern.  These can then get mapped automatically
to the appropriate "type" and often can also be used to set a
conservatively-correct "length".

Thanks,
Richard

Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Segher Boessenkool]

On Tue, Aug 08, 2017 at 10:41:05AM -0600, Jeff Law wrote:
> On 08/05/2017 11:15 AM, Segher Boessenkool wrote:
> > For size cost I currently use just "length", but I haven't looked at
> > size cost much at all yet.
> I think that's fine.  "length" is pretty standardized at this point and
> it's the right metric.  For ports that don't bother defining a length
> attribute, punt in some reasonable manner.

I do this is in the target hook, not in generic code.  Commonizing
two lines of code (at the cost of extra complexity for targets that
do *not* want to handle things that way) is not worth it IMO.


Segher