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?

> --- 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?

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

Yoda conditions have we should not.

> +     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);
      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).


Segher

Reply via email to