| Message ID | AM4PR0701MB21624DB0B4687E4C800404F2E4F20@AM4PR0701MB2162.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On 09/17/2016 05:29 PM, Bernd Edlinger wrote: > On 09/17/16 22:29, Jan-Benedict Glaw wrote: >> On Fri, 2016-09-09 21:40:38 +0000, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >>> Hi, >>> >>> I think it is time to remove support for INITIAL_FRAME_POINTER_OFFSET, which is no longer >>> used by any target today. This removes a bunch of conditional code, and fixes a few bits >>> in the documentation. I'd say that part of the documentation is quite out of sync, but I just >>> have to stop somewhere. >>> >>> >>> Bootstrapped and reg-tested on x86_64-pc-linux.gnu >> >> The vax backend doesn't yet define ELIMINABLE_REGS. >> > > Oh, yes. I see. What a hack. > > Then we should define it. > > But simply returning zero for the fp to sp offset is not ok, > and even if the offset is not used for register eliminations > it should still be correct for rtx_addr_can_trap_p > to know the safe stack frame offset ranges. > > I would assume a small performance improvement, when > rtx_addr_can_trap_p has correct data available. > > How about this patch, it should at least fix the bootstrap. > Is it OK for trunk? OK. jeff
On 09/19/16 23:51, Jeff Law wrote: > On 09/17/2016 05:29 PM, Bernd Edlinger wrote: >> On 09/17/16 22:29, Jan-Benedict Glaw wrote: >>> On Fri, 2016-09-09 21:40:38 +0000, Bernd Edlinger >>> <bernd.edlinger@hotmail.de> wrote: >>>> Hi, >>>> >>>> I think it is time to remove support for >>>> INITIAL_FRAME_POINTER_OFFSET, which is no longer >>>> used by any target today. This removes a bunch of conditional code, >>>> and fixes a few bits >>>> in the documentation. I'd say that part of the documentation is >>>> quite out of sync, but I just >>>> have to stop somewhere. >>>> >>>> >>>> Bootstrapped and reg-tested on x86_64-pc-linux.gnu >>> >>> The vax backend doesn't yet define ELIMINABLE_REGS. >>> >> >> Oh, yes. I see. What a hack. >> >> Then we should define it. >> >> But simply returning zero for the fp to sp offset is not ok, >> and even if the offset is not used for register eliminations >> it should still be correct for rtx_addr_can_trap_p >> to know the safe stack frame offset ranges. >> >> I would assume a small performance improvement, when >> rtx_addr_can_trap_p has correct data available. >> >> How about this patch, it should at least fix the bootstrap. >> Is it OK for trunk? > OK. > jeff Jeff, I am sorry. But I think I got the sign of the elimination offset wrong, because I peeked at a stack-grows-upward target (hppa). And the doc/tm.texi is not a big help either. With my limited testing this did not make any difference. Actually I have not touched any vax since I was a student. And that is already quite a while ago... I hope you don't mind when I commit it this way: +#define INITIAL_ELIMINATION_OFFSET(FROM, TO, OFFSET) \ + ((OFFSET) = get_frame_size ()) Thanks Bernd.
On 09/19/2016 04:24 PM, Bernd Edlinger wrote: > On 09/19/16 23:51, Jeff Law wrote: >> On 09/17/2016 05:29 PM, Bernd Edlinger wrote: >>> On 09/17/16 22:29, Jan-Benedict Glaw wrote: >>>> On Fri, 2016-09-09 21:40:38 +0000, Bernd Edlinger >>>> <bernd.edlinger@hotmail.de> wrote: >>>>> Hi, >>>>> >>>>> I think it is time to remove support for >>>>> INITIAL_FRAME_POINTER_OFFSET, which is no longer >>>>> used by any target today. This removes a bunch of conditional code, >>>>> and fixes a few bits >>>>> in the documentation. I'd say that part of the documentation is >>>>> quite out of sync, but I just >>>>> have to stop somewhere. >>>>> >>>>> >>>>> Bootstrapped and reg-tested on x86_64-pc-linux.gnu >>>> >>>> The vax backend doesn't yet define ELIMINABLE_REGS. >>>> >>> >>> Oh, yes. I see. What a hack. >>> >>> Then we should define it. >>> >>> But simply returning zero for the fp to sp offset is not ok, >>> and even if the offset is not used for register eliminations >>> it should still be correct for rtx_addr_can_trap_p >>> to know the safe stack frame offset ranges. >>> >>> I would assume a small performance improvement, when >>> rtx_addr_can_trap_p has correct data available. >>> >>> How about this patch, it should at least fix the bootstrap. >>> Is it OK for trunk? >> OK. >> jeff > > Jeff, I am sorry. > > But I think I got the sign of the elimination offset wrong, > because I peeked at a stack-grows-upward target (hppa). > And the doc/tm.texi is not a big help either. Funny, I almost called that out as something to double check :-) > > With my limited testing this did not make any difference. > > Actually I have not touched any vax since I was a student. > > And that is already quite a while ago... > > I hope you don't mind when I commit it this way: > +#define INITIAL_ELIMINATION_OFFSET(FROM, TO, OFFSET) \ > + ((OFFSET) = get_frame_size ()) That's fine. jeff
2016-09-17 Bernd Edlinger <bernd.edlinger@hotmail.de> * config/var/vax.h (ELIMINABLE_REGS): Define. (INITIAL_ELIMINATION_OFFSET): Define. Index: gcc/config/vax/vax.h =================================================================== --- gcc/config/vax/vax.h (revision 240215) +++ gcc/config/vax/vax.h (working copy) @@ -333,6 +333,16 @@ } \ while (0) +/* This macro specifies a table of register pairs used to eliminate + unneeded registers that point into the stack frame. */ +#define ELIMINABLE_REGS {{FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM}} + +/* On the VAX, FRAME_POINTER_REQUIRED is always 1, so the definition of this + macro doesn't matter for register eliminations, but it should still + give realistic data for rtx_addr_can_trap_p. */ +#define INITIAL_ELIMINATION_OFFSET(FROM, TO, OFFSET) \ + ((OFFSET) = -get_frame_size ()) + /* EXIT_IGNORE_STACK should be nonzero if, when returning from a function, the stack pointer does not matter. The value is tested only in functions that have frame pointers.