Jeffrey Law - Re: [RTL, ColdFire 24/63] Add support for a MODE_INDEX_REG_CLASS macro (original) (raw)

This is the mail archive of the gcc-patches@gcc.gnu.orgmailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

On Wed, 2007-02-14 at 09:37 +0000, Richard Sandiford wrote:

Jeffrey Law law@redhat.com writes:

On Wed, 2007-01-17 at 05:36 +0000, Richard Sandiford wrote:

The wider issue seems to be: should we either

(a) allow indexed addresses for all modes and use constraints and secondary reloads to handle the cases where they aren't allowed or

(b) prevent the use of index registers for modes if the vast majority of MEMs in those modes are likely to not be index registers. Fundamentally the PA and m68k really need to be rejecting indexed address modes since those modes are not valid addressing modes according to a strict definition of G_I_L_A. The two ports lie about the legitimitacy of such address modes and if they continue to do so, then the details of dealing with the lie really belong in the backend (until such time as someone fixes reload and G_I_L_A to be general enough to handle the kinds of cases that come up on the m68k and PA).

I don't agree. You seem to be saying that G_I_L_A has to be independent of the addressed machine_mode, so that if we forbid index registers for SFmode, we must do it for all modes. Or, conversely, if we allow disp(base,index) addresses for QImode, we must do so for SFmode too. Somewhat. It's legitimate to select based on mode, but you must not confuse GCC's concept of a mode with the register file used on the actual hardware.

You also have to be careful because of how double_reg_address_ok is computed. double_reg_address_ok blindly uses QImode and if we ever get a valid address in QImode, then reload assumes that it can use indexing for any mode.

An alternate (and possibly better) approach to fixing this mess would be to make double_reg_address_ok an array of booleans indexed by mode, then reload could check for validity of double_reg_address_ok for the desired mode rather than assuming that if it's valid for QImode, then it must be valid for other modes.

As I've said before, I think ColdFire's G_I_L_A should continue rejecting indexed addressing modes for SFmode not for correctness, but for optimisation. We want to optimise SFmode code based on the assumption that we will be using the FPU to implement it. We therefore want to expose the lack of indexed addresses early on, so that the pre-reload optimisers and register allocators can see the separate address calculations. Sure, we lose the ability to copy SFmode values around using GPRs and indexed addresses, but the trade-off is IMO a good one. I've got absolutely no problems with rejecting addressing modes in G_I_L_A.

So, from an optimisation perspective, I think it is reasonable to accept indexed addresses for SImode and not for SFmode. I'm still not sure whether you disagree with that. I disagree with that if the port allows integer values in FP regsters and the hardware can't do FP indexed loads/stores because G_I_L_A could be passed SImode but generate an FP load/store.

Now G_I_L_A has a mode argument precisely so that the legitimacy of an address can depend on the mode. However, G_I_L_A is not the only backend interface for describing addressing modes; we also have the BASE_REG_CLASS and INDEX_REG_CLASS macros. These latter macros are all that reload looks at when legitimizing addresses, so they must of course agree with G_I_L_A. I disagree on this point and as I've stated before I think you're using this to avoid fixing the backend to deal with a broken G_I_L_A.

The problem we have at the moment is that, while G_I_L_A and BASE_REG_CLASS are parameterised on the mode (and other things), INDEX_REG_CLASS isn't. So at the moment, it isn't possible for the *_REG_CLASS macros to fully agree with G_I_L_A. My patch is fixing that, and nothing else. When did BASE_REG_CLASS get parameterized on the mode? That seems awful wrong as well. I still don't see any valid need for {BASE,INDEX}_REG_CLASS to be parameterized on the mode of the memory reference.

Can you point me to the discusion for the BASE_REG_CLASS change? [ God I hope I didn't approve that patch :-) ]

Jeff

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]