This is the mail archive of the gcc-patches@gcc.gnu.org mailing 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]

Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers


Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
>> > You also need to do the same thing for TARGET_HARD_REGNO_SCRATCH_OK,
>> > to stop peephole2 from using unsaved registers as scratch registers.
>> >
>> > I should dig out my patches to clean up this interface.  It's just
>> > too brittle to have two macros that say what registers can be used
>> > after reload.
>
> Indeed. I naively thought that there would be a single place that needed
> an update and overlooked this hook. 
>
>> Gosh, I don't like the interface of them at all.  I have interrupt functions
>> and I want all the goodness out of gcc and I want gcc to just know that it
>> can't use registers it doesn't want to save for any reason that the port
>> marked as saved by the calling api of the function.  :-(  Very few ports (3)
>> define TARGET_HARD_REGNO_SCRATCH_OK and many port (19 or so) don't define
>> this but do seem to have interrupt functions.  The existing documentation
>> seems not give be enough information to let me decide if I need to define it
>> or not.  I read through all the ports that do define it, and none of them
>> shed any light on to allow me to decide if my port needs to or not.
>> 
>> So, first question is, are the 16 other ports that have interrupt functions
>> and don't define this just buggy (or non-optimal)?  How can I tell if I need
>> to or not?  A single example of that does cause a port to see it and a single
>> example of why a port would not need to define it would be instructive.

The port is only buggy if they have define_peephole2s with match_scratches
and those match_scratches require a register that would be saved by
an interrupt function.  In other cases defining T_H_R_S_O would have
no effect (but still be a good idea for future proofing -- obviously
someone who adds a new define_peephole2 is unlikely to be thinking
about inerrupt functions).

>> Along the lines of TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS, I'd be
>> willing to explain the required abi of each function, and then I just want
>> gcc to just work. All functions are normal, except for interrupt functions,
>> all allocatable register need to be saved.  I think this is a universally
>> true thing, and it just happens to be true on my port as well.
>> 
>> Anyway, I'd love to see gcc improved in this area.
>> 
>> This one is a pet peeve of mine as I've seen what happens to software when a
>> single register that should have been saved, wasn't.  It was slightly
>> annoying to track down and find.  Fixing was trivial.
>
> The cleanup as Richard suggested would probably be a good start. It's
> interesting that this kind of bug wasn't discovered earlier and likely
> existing for several years.

I think the underlying problem is that target-independent code knows
which set of registers are call(ee)-saved under the ABI, but doesn't
know about function-specific variations.  The idea was to expose that
directly as a new register set in the main function struct.

We can then get rid of all definitions of TARGET_HARD_REGNO_SCRATCH_OK
and most definitions of HARD_REGNO_RENAME_OK.  (Some renames are invalid
for other reasons.)

The main problem is that we then have three sets of registers:
- the ABI call-clobbered set
- the call-clobbered set for the current function
- the set of registers that are clobbered by an already-compiled function
  (for -fipa-ra)

I think that's valid, but you obviously have to be very careful to use
the right one.  Especially when it comes to cached derived information.

Thanks,
Richard


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