Re: [RFC] sched: Do not move expensive insns speculatively (PR68664)

[Andrew Pinski <pinskia at gmail dot com>]

On Thu, Jan 26, 2017 at 4:38 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> Scheduling should never move very expensive instructions to places they
> are executed more frequently.  This patch fixes that, reducing the
> execution time of c-ray by over 40% (I tested on a BE Power7 system).
>
> Is there some existing way to test for "very expensive insn" or "very
> expensive insn we should not speculate"?  Should there be a new hook?
> Is only disallowing (FP) SQRT and DIV a good solution?

Seems like it should be checking the insn cost and compare that
against some parameter.  That is possibly set by the target if needed.

Thanks,
Andrew

>
>
> Segher
>
>
> ---
>  gcc/sched-rgn.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/sched-rgn.c b/gcc/sched-rgn.c
> index 2af3a03..6ccd973 100644
> --- a/gcc/sched-rgn.c
> +++ b/gcc/sched-rgn.c
> @@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "sel-sched.h"
>  #include "tree-pass.h"
>  #include "dbgcnt.h"
> +#include "rtl-iter.h"
>  #include "pretty-print.h"
>  #include "print-rtl.h"
>
> @@ -2147,12 +2148,20 @@ static int
>  can_schedule_ready_p (rtx_insn *insn)
>  {
>    /* An interblock motion?  */
> -  if (INSN_BB (insn) != target_bb
> -      && IS_SPECULATIVE_INSN (insn)
> -      && !check_live (insn, INSN_BB (insn)))
> -    return 0;
> -  else
> -    return 1;
> +  if (INSN_BB (insn) != target_bb && IS_SPECULATIVE_INSN (insn))
> +    {
> +      /* Cannot schedule this insn unless all operands are live.  */
> +      if (!check_live (insn, INSN_BB (insn)))
> +       return 0;
> +
> +      /* Should not move expensive instructions speculatively.  */
> +      subrtx_iterator::array_type array;
> +      FOR_EACH_SUBRTX (iter, array, insn, NONCONST)
> +        if (*iter && GET_CODE (*iter) == SQRT)
> +         return 0;
> +    }
> +
> +  return 1;
>  }
>
>  /* Updates counter and other information.  Split from can_schedule_ready_p ()
> --
> 1.9.3
>