On 11-05-25 6:58 PM, Hans-Peter Nilsson wrote:
On Wed, 25 May 2011, Vladimir Makarov wrote:
This patch solves http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49154 for CRIS.
The problem was in that the pressure classes did not contain SRP register and
the assert failed.
I'm not sure I understand the basic requirement.
It is ambiguous area. Register pressure classes are used for register
pressure evaluation in several parts of the compiler:
register pressure sensitive loop invariant motion
register pressure sensitive insn scheduling
and in IRA to form regions for allocation
In some way register pressure classes are what cover classes were when
they were defined. But right definition of register pressure classes
are not so critical as the right definition of cover classes were
earlier because register pressure calculation is in some way
approximation (even if we know what exact classes will be used for each
pseudo during the allocation, but we don't know it yet exactly when the
register pressure is calculated) because classes used for allocation
(allocno classes) are calculated dynamicly and they can be different
from register pressure classes and the old cover classes and, the most
important thing, the allocno classes can be intersected in any way which
makes the register pressure caclulation is inaccurate. Still the
register pressure calculation is useful.
I added an assertion which checks that the calculated register pressure
classes contains all allocatable hard registers. When the assertion
fails we have this problem. But the compiler will work well even if the
assertion fails. Generally speaking, we could remove the assertion
without harm. The assertion is just a check for *the most* targets that
the pressure classes calculation is not broken because for the most
targets the register pressure classes should contain all available hard
registers.
The assertion failed for CRIS because we had pressure classes only
CC0_REGS and GENERAL_REGS which do not includes SRP register. We cannot
use SPECIAL_REGS for pressure classes because it contains too different
registers (the algorithm finding register pressure classes looks how
moves in the class is costly, e.g. moves from CC0_REGS and SRP which are
part of SPECIAL_REGS is costly). Adding class SRP_REGS, we forms
pressure classes SRP_REGS, CC0_REGS, and GENERAL_REGS which contain all
available hard regs and the assertion becomes true.
So I could remove the assertion or add SRP_REGS class for CRIS. I
prefer the 2nd. Coloring will not change after adding SRP_REGS because
we use dinamicaly calculated classes (simply hard reg sets which are
subsets of the allocno classes) which can be different from classes
defined by the target dependent files.
I hope that the details I wrote here will help to understand the reasons
for the problem but as I wrote it is very ambiguous area.
OK to commit?
2011-05-25 Vladimir Makarov<vmaka...@redhat.com>
PR rtl-optimization/49154
* config/cris/cris.h (reg_class, REG_CLASS_NAMES)
(REG_CLASS_CONTENTS, REGNO_REG_CLASS): Add class SRP_REGS.
Can you please suggest an update to the target macro
documentation to reflect the requirement it's currently failing?
To feel ok with this change I need to understand why it's not ok
as is: I can't see the error, just a random change narrowing a
register class that at a glance *should* not be needed. To wit,
I need to understand why the bug is here and that it's not the
failing assert in IRA that's wrong.
I don't think it is necessary because as I know only CRIS requires
register classes modification. But I'd say the general rule would be
defining more classes as possible (each class for uniform groups of
registers and for different unions of them). It would help to avoid
such problem and improve coloring. On the other hand adding all the
different unions would be probably overkill for the most targets.
(And thanks for your work, really. Don't worry about fitting
the update into texinfo format, I can help with that.)
