[PATCH 8/8] Use rank_for_schedule to as tie-breaker in model_order_p

[Maxim Kuvyrkov]

On Oct 22, 2014, at 9:24 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:

> Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> writes:
>> On Oct 21, 2014, at 9:11 PM, Richard Sandiford
>> <richard.sandiford@arm.com> wrote:
>> 
>>> Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> writes:
>>>> This patch improves model_order_p to use non-reg-pressure version of
>>>> rank_for_schedule when it needs to break the tie.  At the moment it is
>>>> comparing INSN_PRIORITY by itself, and it seems prudent to outsource
>>>> that to rank_for_schedule.
>>> 
>>> Do you have an example of where this helps?  A possible danger is that
>>> rank_for_schedule might (indirectly) depend on state that isn't maintained
>>> or updated in the same way during the model schedule phase.
>> 
>> I don't have an example where this patch helps, and I consider this
>> patch a general cleanup.  From what I can see, all scheduler data
>> structures are maintained during reg_sched_model scheduling.
> 
> I'm not really sure it's a cleanup though.  The code isn't aesthetically
> cleaner because of the way it has to switch the global sched_pressure
> variable for the duration of the call.  And it doesn't really seem
> conceptually cleaner.  The model ordering was deliberately simple
> because the model schedule doesn't take pipeline characteristics like
> issue delays or unit conflicts into account; all it's doing is trying to
> minimise the register pressure.  It isn't obvious to me that all the
> extra stuff that rank_for_schedule does would make sense in that context
> (i.e. that it would tend to reduce pressure compared to the current test,
> rather than increase it).  It also looks like using rank_for_schedule would
> increase compile time.
> 
> So personally I'd rather keep things the way they are unless we have
> a motivating example.

I will not be pushing for this particular change, I'm fine with dropping the patch if current code looks more aesthetically pleasing.

Thanks,

--
Maxim Kuvyrkov
www.linaro.org