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

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> The comparison order for SCHED_PRESSURE_MODEL is incorrect.  If either
> instruction is not in target_bb, the ordering is not well defined.  To fix
> this, give all instructions in target_bb the highest priority and sort all
> other instructions behind it.  This way instructions in target_bb will be
> sorted using the pressure model, and instructions outside it will use
> RFS_DEP_COUNT and/or RFS_TIE for their order.
>
> Bootstrap OK on AArch64, OK for commit?
>
> ChangeLog:
> 2018-01-31  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..2c9dd87426930ee99b2a4c0950cadea96f9553bc 100644
> --- a/gcc/haifa-sched.c
> +++ b/gcc/haifa-sched.c
> @@ -2783,12 +2783,18 @@ 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 (INSN_BB (tmp) == target_bb && INSN_BB (tmp2) == target_bb)
> +	{
> +	  diff = model_index (tmp) - model_index (tmp2);
> +	  gcc_assert (diff != 0);
> +	  return rfs_result (RFS_PRESSURE_INDEX, diff, tmp, tmp2);
> +	}
> +      else if (INSN_BB (tmp) == target_bb)
> +	return rfs_result (RFS_PRESSURE_INDEX, -1, tmp, tmp2);
> +      else if (INSN_BB (tmp2) == target_bb)
> +	return rfs_result (RFS_PRESSURE_INDEX, 1, tmp, tmp2);
>      }

This was the original intent, but was changed in r213708.  TBH I'm not
sure what the second hunk in that revision fixed, since model_index is
supposed to return an index greater than all valid indices when passed
an instruction outside the current block.  Maxim, do you remember?

Thanks,
Richard