| Message ID | 20130207084505.GX5023@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 7, 2013 at 9:45 AM, Alan Modra <amodra@gmail.com> wrote: > I think this one counts as obvious, but I'll ask for permission anyway. > Bootstrapped etc. powerpc-linux. OK everywhere? Isn't there a way to just disable the out-of-line register save/restore functions? Adding -O2 looks odd. On a related note I see the ppc backend uses optimize_size almost everywhere instead of optimize_function_for_size_p () or optimize_insn_for_size_p (). It seems only the i386 backend was properly transitioned here ... Richard. > PR target/45053 > * config/rs6000/t-crtstuff (CRTSTUFF_T_CFLAGS): Add -O2. > > Index: libgcc/config/rs6000/t-crtstuff > =================================================================== > --- libgcc/config/rs6000/t-crtstuff (revision 195839) > +++ libgcc/config/rs6000/t-crtstuff (working copy) > @@ -1,3 +1,6 @@ > # If .sdata is enabled __CTOR_{LIST,END}__ go into .sdata instead of > # .ctors. > -CRTSTUFF_T_CFLAGS = -msdata=none > +# Do not build crtend.o with -Os as that can result in references to > +# out-of-line register save/restore functions, which may be unresolved > +# as crtend.o is linked after libgcc.a. > +CRTSTUFF_T_CFLAGS = -msdata=none -O2 > > -- > Alan Modra > Australia Development Lab, IBM
On Thu, Feb 07, 2013 at 10:11:02AM +0100, Richard Biener wrote: > On Thu, Feb 7, 2013 at 9:45 AM, Alan Modra <amodra@gmail.com> wrote: > > I think this one counts as obvious, but I'll ask for permission anyway. > > Bootstrapped etc. powerpc-linux. OK everywhere? > > Isn't there a way to just disable the out-of-line register save/restore > functions? Adding -O2 looks odd. There isn't. See rs6000.c:rs6000_savres_strategy. I should note too that using -O2 (anything but -Os) is only a solution for powerpc ELF. powerpc-darwin may not be susceptible to the bug, but if it is then some changes will be needed in rs6000_savres_strategy. AIX doesn't have a problem here since it always uses inline gpr saves/restores. Also, powerpc64 targets using GNU ld or gold don't have a problem with crtend.o calling save/restore functions as the linker creates these functions.
On Thu, Feb 7, 2013 at 11:51 AM, Alan Modra <amodra@gmail.com> wrote: > On Thu, Feb 07, 2013 at 10:11:02AM +0100, Richard Biener wrote: >> On Thu, Feb 7, 2013 at 9:45 AM, Alan Modra <amodra@gmail.com> wrote: >> > I think this one counts as obvious, but I'll ask for permission anyway. >> > Bootstrapped etc. powerpc-linux. OK everywhere? >> >> Isn't there a way to just disable the out-of-line register save/restore >> functions? Adding -O2 looks odd. > > There isn't. See rs6000.c:rs6000_savres_strategy. I should note too > that using -O2 (anything but -Os) is only a solution for powerpc ELF. > powerpc-darwin may not be susceptible to the bug, but if it is then > some changes will be needed in rs6000_savres_strategy. AIX doesn't > have a problem here since it always uses inline gpr saves/restores. > > Also, powerpc64 targets using GNU ld or gold don't have a problem with > crtend.o calling save/restore functions as the linker creates these > functions. It should be possible to add a -msavres-inline switch, right? The other option would be to have a -Ono-s option that turns -Os into -O2 (optimize == 2 for both) but leaves -O1/-O0 alone (by basically just setting optimize_size to 0). Of course I consider that more ugly than a new switch to explicitely disable non-inline saveres code ;) Richard. > -- > Alan Modra > Australia Development Lab, IBM
On Thu, Feb 07, 2013 at 12:49:03PM +0100, Richard Biener wrote:
> It should be possible to add a -msavres-inline switch, right?
Sure, but doesn't rs6000 have enough options already?
Perhaps the best solution would be to move __do_global_ctors_aux out
of crtend.o for targets that support hidden global symbols, but that
probably isn't appropriate for gcc's current stage of development.
Also, if you're worried about crtbegin.o and crtend.o size, then it
would help to throw away call_frame_dummy, call___do_global_ctors_aux,
and call___do_global_dtors_aux. Hmm, and we could do that quite
easily by defining CRT_CALL_STATIC_FUNCTION. Done! Another 4.9 patch
queued.
On Feb 7, 2013, at 3:49 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Feb 7, 2013 at 11:51 AM, Alan Modra <amodra@gmail.com> wrote: >> On Thu, Feb 07, 2013 at 10:11:02AM +0100, Richard Biener wrote: >>> On Thu, Feb 7, 2013 at 9:45 AM, Alan Modra <amodra@gmail.com> wrote: >>>> I think this one counts as obvious, but I'll ask for permission anyway. >>>> Bootstrapped etc. powerpc-linux. OK everywhere? >>> >>> Isn't there a way to just disable the out-of-line register save/restore >>> functions? Adding -O2 looks odd. >> >> There isn't. See rs6000.c:rs6000_savres_strategy. I should note too >> that using -O2 (anything but -Os) is only a solution for powerpc ELF. >> powerpc-darwin may not be susceptible to the bug, but if it is then >> some changes will be needed in rs6000_savres_strategy. AIX doesn't >> have a problem here since it always uses inline gpr saves/restores. >> >> Also, powerpc64 targets using GNU ld or gold don't have a problem with >> crtend.o calling save/restore functions as the linker creates these >> functions. > > It should be possible to add a -msavres-inline switch, right? The other > option would be to have a -Ono-s option that turns -Os into -O2 > (optimize == 2 for both) but leaves -O1/-O0 alone (by basically just > setting optimize_size to 0). Of course I consider that more ugly than > a new switch to explicitely disable non-inline saveres code ;) I like the -O2 approach as a permanent solution… Having flags adds a burden to everyone and should be discouraged in all but the most dire of cases. This isn't dire enough.
On Thu, Feb 07, 2013 at 07:15:05PM +1030, Alan Modra wrote: > I think this one counts as obvious, but I'll ask for permission anyway. > Bootstrapped etc. powerpc-linux. OK everywhere? Okay for trunk (and backports, if you want). -O2 may look odd but there is a nice big comment explaining it (please add something like "(see PR45053)" though). Thanks, Segher > PR target/45053 > * config/rs6000/t-crtstuff (CRTSTUFF_T_CFLAGS): Add -O2. > > Index: libgcc/config/rs6000/t-crtstuff > =================================================================== > --- libgcc/config/rs6000/t-crtstuff (revision 195839) > +++ libgcc/config/rs6000/t-crtstuff (working copy) > @@ -1,3 +1,6 @@ > # If .sdata is enabled __CTOR_{LIST,END}__ go into .sdata instead of > # .ctors. > -CRTSTUFF_T_CFLAGS = -msdata=none > +# Do not build crtend.o with -Os as that can result in references to > +# out-of-line register save/restore functions, which may be unresolved > +# as crtend.o is linked after libgcc.a. > +CRTSTUFF_T_CFLAGS = -msdata=none -O2 >
Index: libgcc/config/rs6000/t-crtstuff =================================================================== --- libgcc/config/rs6000/t-crtstuff (revision 195839) +++ libgcc/config/rs6000/t-crtstuff (working copy) @@ -1,3 +1,6 @@ # If .sdata is enabled __CTOR_{LIST,END}__ go into .sdata instead of # .ctors. -CRTSTUFF_T_CFLAGS = -msdata=none +# Do not build crtend.o with -Os as that can result in references to +# out-of-line register save/restore functions, which may be unresolved +# as crtend.o is linked after libgcc.a. +CRTSTUFF_T_CFLAGS = -msdata=none -O2