Re: [PATCH] haifa-sched: fix autopref_rank_for_schedule qsort comparator

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

> On Sep 19, 2017, at 5:25 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> 
> On Tue, 19 Sep 2017, Maxim Kuvyrkov wrote:
>> How about the following:
>> 1. if both instructions are "irrelevant", then return "0".
>> 2. if one instruction is "relevant" and another is "irrelevant", then
>> "relevant" instruction is always greater (or lesser) than the non-relevant. 
>> 3. if both instructions are "relevant", then call autopref_rank_data.
> 
> Sounds good, the following patch implements this (with 'relevant' greater).
> 
>> I don't have immediate answer on whether "relevant" or "irrelevant"
>> instructions should be pushed towards beginning of the ready list.  Possibly,
>> we want to delay "relevant" instructions and push "irrelevant" towards
>> beginning of ready list so that more "relevant" instructions can enter ready
>> list as their dependencies are resolved from scheduling "irrelevant"
>> instructions.
>> 
>> WDYT?
> 
> *nod*, this makes sense.
> 
> (bootstrap/regtest running on x86-64)
> 
> Thanks!
> 
> 	* haifa-sched.c (autopref_rank_for_schedule): Order 'irrelevant' insns
> 	first, always call autopref_rank_data otherwise.
> 
> diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
> index af0ed27b18f..8f006de2319 100644
> --- a/gcc/haifa-sched.c
> +++ b/gcc/haifa-sched.c
> @@ -5707,7 +5707,8 @@ autopref_rank_data (autopref_multipass_data_t data1,
> static int
> autopref_rank_for_schedule (const rtx_insn *insn1, const rtx_insn *insn2)
> {
> -  for (int write = 0; write < 2; ++write)
> +  int r = 0;
> +  for (int write = 0; write < 2 && !r; ++write)
>     {
>       autopref_multipass_data_t data1
> 	= &INSN_AUTOPREF_MULTIPASS_DATA (insn1)[write];
> @@ -5716,21 +5717,20 @@ autopref_rank_for_schedule (const rtx_insn *insn1, const rtx_insn *insn2)
> 
>       if (data1->status == AUTOPREF_MULTIPASS_DATA_UNINITIALIZED)
> 	autopref_multipass_init (insn1, write);
> -      if (data1->status == AUTOPREF_MULTIPASS_DATA_IRRELEVANT)
> -	continue;
> 
>       if (data2->status == AUTOPREF_MULTIPASS_DATA_UNINITIALIZED)
> 	autopref_multipass_init (insn2, write);
> -      if (data2->status == AUTOPREF_MULTIPASS_DATA_IRRELEVANT)
> -	continue;
> 
> -      if (!rtx_equal_p (data1->base, data2->base))
> -	continue;
> +      int irrel1 = data1->status == AUTOPREF_MULTIPASS_DATA_IRRELEVANT;
> +      int irrel2 = data2->status == AUTOPREF_MULTIPASS_DATA_IRRELEVANT;
> 
> -      return autopref_rank_data (data1, data2);
> +      if (!irrel1 && !irrel2)
> +	r = autopref_rank_data (data1, data2);
> +      else
> +	r = irrel2 - irrel1;

I'd like to keep read/write processing balanced.  In the above "read" analysis has greater weight than "write" analysis.  Also, autopref_rank_data() should not be called if !rtx_equal_p (data1->base, data2->base).

How about the attached patch?

--
Maxim Kuvyrkov
www.linaro.org

Attachment: autopref_rank-fix.patch
Description: Binary data