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

[Richard Sandiford <richard dot sandiford at arm dot com>]

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.

Thanks,
Richard