This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Robert Suchanek <Robert dot Suchanek at imgtec dot com>
- Cc: Mike Stump <mikestump at comcast dot net>, "Catherine_Moore\ at mentor dot com" <Catherine_Moore at mentor dot com>, Matthew Fortune <Matthew dot Fortune at imgtec dot com>, "gcc-patches\ at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 15 Aug 2015 11:32:14 +0100
- Subject: Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers
- Authentication-results: sourceware.org; auth=none
- References: <B5E67142681B53468FAF6B7C31356562441B01AD at hhmail02 dot hh dot imgtec dot org> <87k2szc6b9 dot fsf at googlemail dot com> <4438A5B8-6266-4E4F-A43C-8BA503A08900 at comcast dot net> <B5E67142681B53468FAF6B7C31356562441B05D2 at hhmail02 dot hh dot imgtec dot org>
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