This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [mips] Return correct value from TARGET_SCHED_REORDER2 (PR rtl-optimization/37360)
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Jie Zhang <jie at codesourcery dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Andrey Belevantsev <abel at ispras dot ru>
- Date: Wed, 20 Oct 2010 21:44:36 +0100
- Subject: Re: [mips] Return correct value from TARGET_SCHED_REORDER2 (PR rtl-optimization/37360)
- References: <4CBF02B4.4010207@codesourcery.com>
Jie Zhang <jie@codesourcery.com> writes:
> Currently TARGET_SCHED_REORDER2 returns mips_issue_rate (), which will
> reset can_issue_more to the issue_rate after scheduling each
> instruction. See haifa-sched.c:sched_block for the details. Because of
> this, issue_rate is not honored when scheduling. This caused PR
> rtl-optimization/37360 since the issue_rate is set to a value smaller
> than the number of instructions that scheduler DFA can issue in one
> cycle for mips sb1.
The haifa-sched.c part looks good to me, but I can't approve it.
Thanks for tracking down the underlying problem.
> This patch makes mips target follow rs6000 and sh. It caches
> can_issue_more in TARGET_SCHED_VARIABLE_ISSUE and return the cached
> value in TARGET_SCHED_REORDER2. It also addes the test case from that PR
> and revert the the original hack.
The MIPS parts are OK regardless of whether the haifa-sched.c patch is.
I have to say though that the hook interface is less than ideal if we
have to introduce hacks like this. We ought to pass can_issue_more
to sched_reorder2 if the hook needs to be aware of the current value.
The md.texi documentation implies that sched_reorder2 returns the
same thing as sched_reorder, but as you point out, it doesn't.
The sched_reorder documentation explicitly says that "issue_rate" is
usually the correct return value for that hook, but it seems that
issue_rate is never the correct return value for sched_reorder2.
Having a different prototype for sched_reorder2 (passing the extra
can_issue_more parameter) would IMO help to make this clearer.
Richard