Re: [PATCH] PR84068: Fix sort order of SCHED_PRESSURE_MODEL

[Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>]

> On Feb 2, 2018, at 7:40 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> 
> Right, so here is version 2 which ends up much simpler:
> 
> The comparison function for SCHED_PRESSURE_MODEL is incorrect.  If either
> instruction is not in target_bb, the ordering is not well defined.  
> Since all instructions outside the target_bb get the highest model_index,
> all we need to do is sort on model_index.  If the model_index is the same
> we defer to RFS_DEP_COUNT and/or RFS_TIE.
> 
> Bootstrap OK, OK for commit?

Looks good to me.

--
Maxim Kuvyrkov
www.linaro.org

> 
> ChangeLog:
> 2018-02-02  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	PR rlt-optimization/84068
> 	* haifa-sched.c (rank_for_schedule): Fix SCHED_PRESSURE_MODEL sorting.
> 
> 	PR rlt-optimization/84068
> 	* gcc.dg/pr84068.c: New test.
> --
> 
> diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
> index ebdec46bf04f1ba07e8b70607602a3bc9e7ec7de..4a899b56173dd7d67db084ed86d24ad865ccadcd 100644
> --- a/gcc/haifa-sched.c
> +++ b/gcc/haifa-sched.c
> @@ -2783,12 +2783,11 @@ rank_for_schedule (const void *x, const void *y)
>     }
> 
>   /* Prefer instructions that occur earlier in the model schedule.  */
> -  if (sched_pressure == SCHED_PRESSURE_MODEL
> -      && INSN_BB (tmp) == target_bb && INSN_BB (tmp2) == target_bb)
> +  if (sched_pressure == SCHED_PRESSURE_MODEL)
>     {
>       diff = model_index (tmp) - model_index (tmp2);
> -      gcc_assert (diff != 0);
> -      return rfs_result (RFS_PRESSURE_INDEX, diff, tmp, tmp2);
> +      if (diff != 0)
> +	return rfs_result (RFS_PRESSURE_INDEX, diff, tmp, tmp2);
>     }
> 
>   /* Prefer the insn which has more later insns that depend on it.
> diff --git a/gcc/testsuite/gcc.dg/pr84068.c b/gcc/testsuite/gcc.dg/pr84068.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..13110d84455f20edfc50f09efe4074721bd6a7d0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr84068.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-sched-critical-path-heuristic -fno-sched-rank-heuristic --param=max-sched-extend-regions-iters=5 --param sched-pressure-algorithm=2" } */
> +
> +#ifdef __SIZEOF_INT128__
> +typedef __int128 largeint;
> +#else
> +typedef long long largeint;
> +#endif
> +
> +largeint a;
> +int b;
> +
> +largeint
> +foo (char d, short e, int f)
> +{
> +  b = __builtin_sub_overflow_p (b, 1, (unsigned long)0);
> +  return a + f;
> +}