Patchwork [RS6000] Fix PR45053

login
register
mail settings
Submitter Alan Modra
Date Feb. 7, 2013, 8:45 a.m.
Message ID <20130207084505.GX5023@bubble.grove.modra.org>
Download mbox | patch
Permalink /patch/218866/
State New
Headers show

Comments

Alan Modra - Feb. 7, 2013, 8:45 a.m.
I think this one counts as obvious, but I'll ask for permission anyway.
Bootstrapped etc. powerpc-linux.  OK everywhere?

	PR target/45053
	* config/rs6000/t-crtstuff (CRTSTUFF_T_CFLAGS): Add -O2.
Richard Guenther - Feb. 7, 2013, 9:11 a.m.
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
Alan Modra - Feb. 7, 2013, 10:51 a.m.
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.
Richard Guenther - Feb. 7, 2013, 11:49 a.m.
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
Alan Modra - Feb. 7, 2013, 1:15 p.m.
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.
Mike Stump - Feb. 7, 2013, 6:14 p.m.
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.

Patch

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