Questions about selective scheduler and PowerPC

[Jie Zhang]

On 10/19/2010 10:16 PM, Andrey Belevantsev wrote:
> On 19.10.2010 17:57, Jie Zhang wrote:
>> Removing the failing assert fixes the test case. But I wonder why not
>> just
>> get max_issue correct. I'm testing the attached patch. IMHO, max_issue
>> looks confusing.
>>
>> * The concept of ISSUE POINT has never been used since the code landed in
>> repository.
>>
>> * In the comment just before the function, it's mentioned that MAX_POINTS
>> is the sum of points of all instructions in READY. But it does not match
>> the code. The code only summarizes the points of the first MORE_ISSUE
>> instructions. If later ISSUE_POINTS become not uniform, that piece of
>> code
>> should be redesigned.
>>
>> So I think it's good to remove it now. And "top - choice_stack" is a good
>> replacement for top->n. So we can remove field n from struct
>> choice_entry,
>> too.
>>
>> Now I'm looking at MIPS target to find out why this change in the would
>> cause PR37360.
> I agree that ISSUE_POINTS can be removed, as it was not used (maybe
> Maxim can comment more on this). However, the assert is not about the
> points but exactly about the situation when a target is lying to the
> compiler about its issue rate.
>
> The ideal situation is that we agree on that this should never happen,
> but then you need to fix all targets that use this trick, and it seems
> that there is at least mips, ppc, and x86-64 (which is why I pointed you
> to 45352). The fix would be to find out why claiming the true issue rate
> degrades performance and to implement the proper scheduling hooks for
> changing priority of some insns, or to enable -fsched-pressure for the
> offending targets.
>
I agree. But I still have a question about TARGET_SCHED_ISSUE_RATE. 
According to my understanding of gccint:

[quote]
Target Hook: int TARGET_SCHED_ISSUE_RATE (void)
[snip]
Although the insn scheduler can define itself the possibility of issue 
an insn on the same cycle, the value can serve as an additional 
constraint to issue insns on the same simulated processor cycle
[snip]
[/quote]

it should be allowed to be defined smaller than the issue rate defined 
by the scheduler DFA. So even if the backend defines a DFA which is 
capable to issue 4 instructions in one cycle but it also defines 
TARGET_SCHED_ISSUE_RATE to 3, the scheduler should restrict the number 
of instructions issued in one cycle to 3 instead of 4.

So I think this assert should hold even the backend lies to scheduler 
about the issue rate. Fixing the lies is another problem.

With the attached draft patch, we can enable the assert in max_issue 
without regression on PR37360.

> This is a lot of work, which is why this assert was installed in
> max_issue for relatively short amount of time. Maybe it's time to try
> again, but let's have a consensus first that this assert should never
> trigger by design and we have enough flexibility in the scheduler to
> provide legal means to achieve the same performance effect.
>
Agree.


Regards,
-- 
Jie Zhang
CodeSourcery
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ttt-gcc-mips-sched-reorder2.diff
Type: text/x-patch
Size: 2589 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20101019/e38eadcf/attachment.bin>