On Mon, 2009-11-09 at 14:13 +0000, Ian Bolton wrote:
> Dave Hudson wrote:
> > I've been working on gcc for an ISA (Ubicom32) that seems to have some
> > similarities to the problem you're seeing (we have some regs that can
> > be
> > used for many things but not all) and was seeing a ton a pointless
> > moves
> > introduced by reload.
> >
> > I've ended up trying quite a few different strategies including
> > defining
> > different cover classes and the strategy we're now using is to leave
> > the
> > cover classes the same way you have them but found that the most
> > critical thing was to ensure that REGNO_REG_CLASS was returning a
> > minimal class correctly. Once I had this right then IRA started to do
> > the right thing most of the time (-Os still isn't great but -O2 usually
> > looks very good now). We still see some problems but they're usually a
> > result of reload messing things up or suboptimal handling of
> > auto-incrementing in MEMs.
>
> Thanks for the info, Dave.
>
> Given that C_REGS = r0-r31, BOTTOM_REGS = r0-r15, TOP_CREGS = r16-r31,
> when you say "minimal class", does that mean that I should change my
> macro from this:
>
> /* Map a register number to a register class. */
> #define REGNO_REG_CLASS(REGNO) \
> (BOTTOM_C_REG_P (REGNO) ? BOTTOM_REGS : \
> (REGNO) == LINK_POINTER_REGNUM ? LINK_REGS : \
> C_REG_P (REGNO) ? C_REGS : \
> D_REG_P (REGNO) ? D_REGS : \
> CSR_REG_P (REGNO) ? CSR_REGS : \
> (unsigned)(REGNO) < FIRST_PSEUDO_REGISTER ? INTERNAL_REGS : ALL_REGS)
>
> to this (see C_REG_P line for change):
>
> /* Map a register number to a register class. */
> #define REGNO_REG_CLASS(REGNO) \
> (BOTTOM_C_REG_P (REGNO) ? BOTTOM_REGS : \
> (REGNO) == LINK_POINTER_REGNUM ? LINK_REGS : \
> C_REG_P (REGNO) ? TOP_CREGS : \
> D_REG_P (REGNO) ? D_REGS : \
> CSR_REG_P (REGNO) ? CSR_REGS : \
> (unsigned)(REGNO) < FIRST_PSEUDO_REGISTER ? INTERNAL_REGS : ALL_REGS)
>
> I made the change and nothing noticeable happened, but maybe once I fix
> whatever else needs fixing then the second version of the macro will be
> better?
I think this looks more like what I had to do. FWIW I found it easier
to use an array in our port rather than using a cascading conditional
sequence (I can't remember which port I got that idea from):
#define REG_CLASS_CONTENTS \
{ \
{0x00000000, 0x00000000}, /* No regs */ \
{0x0000ffff, 0x00000000}, /* DATA_REGS */ \
{0x00010000, 0x00000000}, /* FDPIC_REG */ \
{0x20fe0000, 0x00000000}, /* ADDRESS_REGS */ \
{0x20ff0000, 0x00000000}, /* ALL_ADDRESS_REGS */ \
{0x0a000000, 0x00000000}, /* ACC_LO_REGS */ \
{0x0f000000, 0x00000000}, /* ACC_REGS */ \
{0x40000000, 0x00000000}, /* CC_REG */ \
{0x0f00ffff, 0x00000000}, /* DATA_ACC_REGS */ \
{0x10000000, 0x00000000}, /* SOURGE3_REG */ \
{0x80000000, 0x0000007f}, /* SPECIAL_REGS */ \
{0xbfffffff, 0x0000007f}, /* GENERAL_REGS */ \
{0xbfffffff, 0x0000007f} /* ALL_REGS */ \
}
extern enum reg_class const
ubicom32_regclass_map[FIRST_PSEUDO_REGISTER];
/* A C expression whose value is a register class containing hard
register
REGNO. In general there is more than one such class; choose a class
which
is "minimal", meaning that no smaller class also contains the
register. */
#define REGNO_REG_CLASS(REGNO) (ubicom32_regclass_map[REGNO])
and:
enum reg_class const ubicom32_regclass_map[FIRST_PSEUDO_REGISTER] =
{
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
DATA_REGS,
FDPIC_REG,
ADDRESS_REGS,
ADDRESS_REGS,
ADDRESS_REGS,
ADDRESS_REGS,
ADDRESS_REGS,
ADDRESS_REGS,
ADDRESS_REGS,
ACC_REGS,
ACC_LO_REGS,
ACC_REGS,
ACC_LO_REGS,
SOURCE3_REG,
ADDRESS_REGS,
NO_REGS, /* CC_REG must be NO_REGS */
SPECIAL_REGS,
SPECIAL_REGS,
SPECIAL_REGS,
SPECIAL_REGS,
SPECIAL_REGS,
SPECIAL_REGS,
SPECIAL_REGS,
SPECIAL_REGS
};
