| Submitter | Olivier Hainque |
|---|---|
| Date | Sept. 23, 2010, 8:06 a.m. |
| Message ID | <20100923080646.GA7817@cardhu.act-europe.fr> |
| Download | mbox | patch |
| Permalink | /patch/65508/ |
| State | New |
| Headers | show |
Comments
Hello, Ping for http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01838.html Olivier Hainque wrote: > [on powerpc-aix, with -mminimal-toc] ... r30 is not saved/restored > as it should in the sequence triggered by __builtin_unwind_init / > __builtin_eh_return Thanks much in advance, With Kind Regards, Olivier
Hello, Ping # 2 for http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01838.html Olivier Hainque wrote: > [on powerpc-aix, with -mminimal-toc] ... r30 is not saved/restored > as it should in the sequence triggered by __builtin_unwind_init / > __builtin_eh_return Thanks much in advance, With Kind Regards, Olivier
On Mon, Oct 18, 2010 at 4:02 AM, Olivier Hainque <hainque@adacore.com> wrote: > Hello, > > Ping # 2 for > > http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01838.html > > Olivier Hainque wrote: >> [on powerpc-aix, with -mminimal-toc] ... r30 is not saved/restored >> as it should in the sequence triggered by __builtin_unwind_init / >> __builtin_eh_return Okay. Thanks, David
On Wed, Oct 06, 2010 at 10:50:51PM +0200, Olivier Hainque wrote: > Hello, > > Ping for > > http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01838.html > > Olivier Hainque wrote: > > [on powerpc-aix, with -mminimal-toc] ... r30 is not saved/restored > > as it should in the sequence triggered by __builtin_unwind_init / > > __builtin_eh_return This affects powerpc64-linux too. FWIW, the patch looks good to me except for the testsuite addition, which needs some tweaks. I think you'll need to dg-skip-if darwin, add -mno-multiple to the flags, and scan for "st\\[wd\\] 30," so that 64-bit targets pass.
Hello, Alan Modra wrote: > This affects powerpc64-linux too. FWIW, the patch looks good to me > except for the testsuite addition, which needs some tweaks. I think > you'll need to dg-skip-if darwin, add -mno-multiple to the flags, and > scan for "st\\[wd\\] 30," so that 64-bit targets pass. OK. After David's approval (thanks :), I have installed with the test adjustments you suggested. I was not able to verify that they do have the intended effect for ppc64-linux. Thanks for your feedback, Olivier
Patch
--- config/rs6000/rs6000.c (revision 163943) +++ config/rs6000/rs6000.c (working copy) @@ -19584,8 +19591,13 @@ static bool rs6000_reg_live_or_pic_offset_p (int reg) { - return ((df_regs_ever_live_p (reg) + /* If the function calls eh_return, claim used all the registers that would + be checked for liveness otherwise. This is required for the PIC offset + register with -mminimal-toc on AIX, as it is advertised as "fixed" for + register allocation purposes in this case. */ + + return (((crtl->calls_eh_return || df_regs_ever_live_p (reg)) && (!call_used_regs[reg] || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && TARGET_TOC && TARGET_MINIMAL_TOC))) --- testsuite/gcc.target/powerpc/ehreturn.c (revision 0) +++ testsuite/gcc.target/powerpc/ehreturn.c (revision 0) @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mminimal-toc" } */ + +void foo () +{ + long l; void *p; + volatile int x; + + __builtin_unwind_init (); + x = 12; + __builtin_eh_return (l, p); +} + +/* { dg-final { scan-assembler "stw 30," } } */
Hello, Late experiments with -mminimal-toc on ppc-aix revealed a problem with restores of r30 on exception propagation. The register indeed is not saved/restored as it should in the sequence triggered by __builtin_unwind_init/__builtin_eh_return, as visible from compiling the testcase below: void foo () { long l; void *p; volatile int x; __builtin_unwind_init (); x = 12; __builtin_eh_return (l, p); } cc1 -O2 -mminimal-toc foo.c .foo: ... stw 28,128(1) stw 29,132(1) stw 31,140(1) ... To determine whether a register is to be saved/restores, the rs6000 back-end resorts to /* Determine whether the gp REG is really used. */ static bool rs6000_reg_live_or_pic_offset_p (int reg) { return ((df_regs_ever_live_p (reg) && (!call_used_regs[reg] || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && TARGET_TOC && TARGET_MINIMAL_TOC))) || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM && ((DEFAULT_ABI == ABI_V4 && flag_pic != 0) || (DEFAULT_ABI == ABI_DARWIN && flag_pic)))); } which always checks for liveness on AIX. __builtin_unwind_init extends the set of registers to include with: reload () ... if (crtl->saves_all_registers) for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) if (! call_used_regs[i] && ! fixed_regs[i] && ! LOCAL_REGNO (i)) df_set_regs_ever_live (i, true); but r30 isn't included in the set with -mminimal-toc because of rs6000_conditional_register_usage () ... if (TARGET_TOC && TARGET_MINIMAL_TOC) fixed_regs[RS6000_PIC_OFFSET_TABLE_REGNUM] = call_used_regs[RS6000_PIC_OFFSET_TABLE_REGNUM] = 1; The attached patch fixes this by adjusting the rs6000 predicate to claim "really used" for eh_return all the registers that are checked for liveness in the regular case. Bootstrapped and regression tested on powerpc-ibm-aix5.3.0. OK ? Thanks in advance, Olivier 2010-09-23 Olivier Hainque <hainque@adacore.com> * config/rs6000/rs6000.c (rs6000_reg_live_or_pic_offset_p): If the current function calls eh_return, claim live all registers that we need to check for liveness otherwise. testsuite/ * gcc.target/powerpc (ehreturn.c): New test.