Re: Questions about selective scheduler and PowerPC

[Andrey Belevantsev <abel at ispras dot ru>]

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.

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.

Andrey



> /* ??? We used to assert here that we never issue more insns than issue_rate.
> However, some targets (e.g. MIPS/SB1) claim lower issue rate than can be
> achieved to get better performance. Until these targets are fixed to use
> scheduler hooks to manipulate insns priority instead, the assert should
> - be disabled.
> -
> - gcc_assert (more_issue >= 0); */
> + be disabled. */