Message ID | 32da4d9c196bd30988776e19c4f210381b630c92.camel@redhat.com |
---|---|
State | New |
Headers | show |
Series | [committed] Fix STATIC_CHAIN_REGNUM for v850 port | expand |
On Sat, 29 Feb 2020, Jeff Law wrote: > > Wow, I think I wrote the v850 port back in circa 1997 and this bug has been > latent all this time. Vlad's IRA changes twiddled register allocation in just > the right way to expose this bug. > > I'm not sure what I was thinking, but apparently I made a spectacularly bad > choice for the STATIC_CHAIN_REGNUM in choosing a call-saved register (r20). > > It's simply wrong to use a call-saved register for the static chain. Heh. I did that mistake too, for CRIS. :/ A comment from RTH below my (incorrect) comment in cris.c above cris_asm_trampoline_template alludes to there being an ABI-neutral solution: "??? See the i386 regparm=3 implementation that pushes the static chain value to the stack in the trampoline, and uses a call-saved register when called directly." ... but IIRC it either didn't apply for CRIS or I didn't look into it thoroughly enough. Or that's also buggy. brgds, H-P PS. Perhaps a doc update with a warning is a suitable penance? :)
On Tue, 2020-03-03 at 13:38 -0500, Hans-Peter Nilsson wrote: > On Sat, 29 Feb 2020, Jeff Law wrote: > > Wow, I think I wrote the v850 port back in circa 1997 and this bug has been > > latent all this time. Vlad's IRA changes twiddled register allocation in > > just > > the right way to expose this bug. > > > > I'm not sure what I was thinking, but apparently I made a spectacularly bad > > choice for the STATIC_CHAIN_REGNUM in choosing a call-saved register (r20). > > > > It's simply wrong to use a call-saved register for the static chain. > > Heh. I did that mistake too, for CRIS. :/ > > A comment from RTH below my (incorrect) comment in cris.c above > cris_asm_trampoline_template alludes to there being an > ABI-neutral solution: "??? See the i386 regparm=3 implementation > that pushes the static chain value to the stack in the > trampoline, and uses a call-saved register when called > directly." ... but IIRC it either didn't apply for CRIS or I > didn't look into it thoroughly enough. Or that's also buggy. > > brgds, H-P > PS. Perhaps a doc update with a warning is a suitable penance? :) That was already the plan. I took care of the gcc-10 changes.html bit today. jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 7d95db8623f..2a69c680a9b 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2020-02-28 Jeff Law <law@redhat.com> + + * config/v850/v850.h (STATIC_CHAIN_REGNUM): Change to r19. + * config/v850/v850.c (v850_asm_trampoline_template): Update + accordingly. + 2020-02-28 Michael Meissner <meissner@linux.ibm.com> PR target/93937 diff --git a/gcc/config/v850/v850.c b/gcc/config/v850/v850.c index 074adf87687..4b0e28c1786 100644 --- a/gcc/config/v850/v850.c +++ b/gcc/config/v850/v850.c @@ -2961,7 +2961,7 @@ static void v850_asm_trampoline_template (FILE *f) { fprintf (f, "\tjarl .+4,r12\n"); - fprintf (f, "\tld.w 12[r12],r20\n"); + fprintf (f, "\tld.w 12[r12],r19\n"); fprintf (f, "\tld.w 16[r12],r12\n"); fprintf (f, "\tjmp [r12]\n"); fprintf (f, "\tnop\n"); diff --git a/gcc/config/v850/v850.h b/gcc/config/v850/v850.h index 823bc5e17e3..7ae583c7df2 100644 --- a/gcc/config/v850/v850.h +++ b/gcc/config/v850/v850.h @@ -438,8 +438,9 @@ enum reg_class /* Base register for access to arguments of the function. */ #define ARG_POINTER_REGNUM 35 -/* Register in which static-chain is passed to a function. */ -#define STATIC_CHAIN_REGNUM 20 +/* Register in which static-chain is passed to a function. + This must be a call used register. */ +#define STATIC_CHAIN_REGNUM 19 /* If defined, this macro specifies a table of register pairs used to eliminate unneeded registers that point into the stack frame. If