On 12.07.2017 14:11, Segher Boessenkool wrote:
Hi,

On Tue, Jul 11, 2017 at 10:47:27AM +0200, Georg-Johann Lay wrote:
This small addition improves costs of PARALLELs in
rtlanal.c:seq_cost().  Up to now, these costs are
assumed to be 1 which gives gross inexact costs for,
e.g. divmod which is represented as PARALLEL.

insn_rtx_cost returns 0 ("unknown") for such a PARALLEL, so your
current patch does not change this at all?

Huh?  It returns the costs of 1st SET in a PARALLEL (provided it
has one), no?  Or even costs for come compares.


--- rtlanal.c   (revision 248745)
+++ rtlanal.c   (working copy)
@@ -5300,6 +5300,9 @@ seq_cost (const rtx_insn *seq, bool spee
        set = single_set (seq);
        if (set)
          cost += set_rtx_cost (set, speed);

So, why does this not use insn_rtx_cost as well?

You'll have to ask the author of that line...

And I didn't intend to change existing behaviour except for a
case where I know that "1" is far off the real costs.


+      else if (INSN_P (seq)
+              && PARALLEL == GET_CODE (PATTERN (seq)))

Yoda conditions have we should not.

hmm, I didn't find something like PARALLEL_P (rtx).
Is comparing rtx_codes deprecated now?

+       cost += 1 + insn_rtx_cost (PATTERN (seq), speed);
        else
          cost++;
      }

This whole thing could be something like

   if (INSN_P (seq))
     {
       int t = insn_rtx_cost (PATTERN (seq), speed);

This will behave differently.  The original set_src_cost calls
rtx_costs with SET and outer_code = INSN, insn_rtx_cost does not.

My intentions was to be conservative and not silently introduce
performance degradations in whatever back-end by changing the
seen RTXes or codes.

Everything that rtx_costs was called for will be the same.
Nothing changes, just some new calls are added.  But neither are
existing calls removed nor are ones changes to up different
arguments.


       cost += t ? t : COST_N_INSNS (1);
     }
   else
     cost++;

But set_rtx_cost does *not* return the same answer as insn_rtx_cost.

(Why do you need a check for INSN_P here?  Why does it increment the

cost for non-insns?  So many questions).

Again, you'll have to ask the original author for reasoning.

Johann



Segher


Reply via email to