This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Committed][PR37360] Restore MIPS bootstrap
Andrey Belevantsev <abel@ispras.ru> writes:
> Richard Sandiford wrote:
>> Andrey Belevantsev <abel@ispras.ru> writes:
>>> As the patch passed bootstrap on x86_64 and restored MIPS bootstrap, and
>>> it only removes an assert, I think it qualifies as obvious given the
>>> above explanation. I have committed the patch.
>>
>> I'm not sure it counts as obviously correct, given that it was
>> queried in bugzilla.
> I'm sorry if this looked like I ignored your comment -- I have waited
> for a day before committing and decided that you were satisfied with my
> answer.
Yeah, I'm afraid it was unfortunate timing. I don't have much time for
gcc stuff these days, so I didn't get chance to read your message on the
day you sent it.
>> You said:
>>
>>> (In reply to comment #16)
>>>> Could you explain why max_issue() should do anything
>>>> when more_issue <= 0? I'd have expected it to early-out.
>>
>>> But the whole point of the patch is that we _can_ actually issue more insns
>>> even when more_issue <= 0, because the backend is claiming lower issue_rate
>>> than can be achieved (see comment #11). We can return 0 from max_issue when
>>> more_issue <= 0, but I don't see offhand how the theoretical value of 4 issued
>>> insns can then be achieved for sb1.
>>
>> As you say, the SB1 scheduler is deliberately requesting a lower issue
>> rate than can be achieved from the DFA, because its author found that
>> doing so gave better results. So I'm not convinced it's correct to
>> simply ignore the hook in this situation. An early exit of the form:
>>
>> if (more_issue <= 0)
>> return 0;
>>
>> still seems more natural to me. Why have the hook if we're not going
>> to take notice of it?
> I was not clear enough when explaining the issue. SB1 claims that
> issue_rate is 3, when it is actually 4. But, there are situations when
> the rate of 4 can be achieved, like the one in the bug report. Before
> the merge, max_issue would not notice that issue_rate is achieved and
> try to issue more insns. In the case when 4 insns can be issued,
> max_issue will succeed. The actual number of issued insns will be four.
>
> This is the pre-merge behavior.
Ah, sorry, I'd missed that it was the pre-merge behaviour. In that case,
I agree that your patch is what the author of the SB1 patch probably wanted.
Sorry for the noise.
Richard