This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH 8/8] Use rank_for_schedule to as tie-breaker in model_order_p
- From: Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>
- To: Richard Sandiford <richard dot sandiford at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Vladimir Makarov <vmakarov at redhat dot com>
- Date: Wed, 22 Oct 2014 09:27:04 +1300
- Subject: Re: [PATCH 8/8] Use rank_for_schedule to as tie-breaker in model_order_p
- Authentication-results: sourceware.org; auth=none
- References: <9F97F273-F5DC-4E85-8298-4790ED2C9E82 at linaro dot org> <87r3y1kfjk dot fsf at e105548-lin dot cambridge dot arm dot com> <471F5AB0-26C8-4BE9-9699-FB7696217C6A at linaro dot org> <8738ahmard dot fsf at e105548-lin dot cambridge dot arm dot com>
On Oct 22, 2014, at 9:24 AM, Richard Sandiford <email@example.com> wrote:
> Maxim Kuvyrkov <firstname.lastname@example.org> writes:
>> On Oct 21, 2014, at 9:11 PM, Richard Sandiford
>> <email@example.com> wrote:
>>> Maxim Kuvyrkov <firstname.lastname@example.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.