diff mbox

Disable -fuse-caller-save when -pg is active

Message ID B580B8E56670DB4993CF59CB657624A84D6108DF@BADAG02.ba.imgtec.org
State New
Headers show

Commit Message

Radovan Obradovic Nov. 14, 2014, 5:10 p.m. UTC 
Thank you for the quick reply.

> Please repost after updating to test HAVE_prologue and HAVE_epilogue and
> adding a testcase.

I have managed to reproduce the problem on the small test case on mips32, but the test is architecture independent and should probably fail on many other ports without this patch. The optimization is also disabled if macros HAVE_prologue and HAVE_epilogue are not defined or have false value.

Radovan

Patch -

Comments

Jeff Law Nov. 17, 2014, 9:18 p.m. UTC | #1 
On 11/14/14 10:10, Radovan Obradovic wrote:
> Thank you for the quick reply.
>
>> Please repost after updating to test HAVE_prologue and
>> HAVE_epilogue and adding a testcase.
>
> I have managed to reproduce the problem on the small test case on
> mips32, but the test is architecture independent and should probably
> fail on many other ports without this patch. The optimization is also
> disabled if macros HAVE_prologue and HAVE_epilogue are not defined or
> have false value.
Thanks.  This looks good.  I forgot to ask in my prior message, has this 
patch been bootstrapped and regression tested?  If so, on what platform?

Jeff
Radovan Obradovic Dec. 18, 2014, 4 p.m. UTC | #2 
Patch has been tested with DejaGnu gcc test suite on mips32r2 cross compiler and bootstrapped and tested on
x86_64 native compiler.

Radovan
Moore, Catherine Jan. 5, 2015, 2:48 p.m. UTC | #3 
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Radovan Obradovic
> Sent: Thursday, December 18, 2014 11:01 AM
> To: Jeff Law; gcc-patches@gcc.gnu.org
> Cc: Petar Jovanovic
> Subject: RE: [PATCH] Disable -fuse-caller-save when -pg is active
> 
> 
> Patch has been tested with DejaGnu gcc test suite on mips32r2 cross
> compiler and bootstrapped and tested on
> x86_64 native compiler.
> 
Thanks for finishing up the testing.  Would you like me to check this in for you?

> ________________________________________
> From: Jeff Law [law@redhat.com]
> Sent: Monday, November 17, 2014 1:18 PM
> To: Radovan Obradovic; gcc-patches@gcc.gnu.org
> Cc: Petar Jovanovic
> Subject: Re: [PATCH] Disable -fuse-caller-save when -pg is active
> 
> On 11/14/14 10:10, Radovan Obradovic wrote:
> > Thank you for the quick reply.
> >
> >> Please repost after updating to test HAVE_prologue and HAVE_epilogue
> >> and adding a testcase.
> >
> > I have managed to reproduce the problem on the small test case on
> > mips32, but the test is architecture independent and should probably
> > fail on many other ports without this patch. The optimization is also
> > disabled if macros HAVE_prologue and HAVE_epilogue are not defined or
> > have false value.
> Thanks.  This looks good.  I forgot to ask in my prior message, has this patch
> been bootstrapped and regression tested?  If so, on what platform?
> 
> Jeff
Jeff Law Jan. 5, 2015, 5:06 p.m. UTC | #4 
On 01/05/15 07:48, Moore, Catherine wrote:
>
>
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Radovan Obradovic
>> Sent: Thursday, December 18, 2014 11:01 AM
>> To: Jeff Law; gcc-patches@gcc.gnu.org
>> Cc: Petar Jovanovic
>> Subject: RE: [PATCH] Disable -fuse-caller-save when -pg is active
>>
>>
>> Patch has been tested with DejaGnu gcc test suite on mips32r2 cross
>> compiler and bootstrapped and tested on
>> x86_64 native compiler.
>>
> Thanks for finishing up the testing.  Would you like me to check this in for you?
Please do.  My recollection was that the patch itself looked fine, but 
that it needed to go through the usual testing cycle.

jeff
Hans-Peter Nilsson Jan. 6, 2015, 4:01 a.m. UTC | #5 
On Fri, 14 Nov 2014, Radovan Obradovic wrote:
> index eb37bfe..ddaf8e0 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c

> @@ -1605,6 +1612,11 @@ process_options (void)
>    /* Save the current optimization options.  */
>    optimization_default_node = build_optimization_node (&global_options);
>    optimization_current_node = optimization_default_node;
> +
> +  /* Disable use caller save optimization if profiler is active or port
> +     does not emit prologue and epilogue as RTL.  */
> +  if (profile_flag || !HAVE_prologue || !HAVE_epilogue)
> +    flag_use_caller_save = 0;
>  }

This seems wrong.  Why disable caller-save regardless of
profile_flag; is there some long-standing bug in caller-save for
"old" targets?  I guess you want:

> +  if (profile_flag && (!HAVE_prologue || !HAVE_epilogue))


Not that it matter too much, I don't think we care about such
targets.  I don't even bother to grep if there is any. :)

brgds, H-P
Jeff Law Jan. 15, 2015, 6:33 p.m. UTC | #6 
On 01/05/15 21:01, Hans-Peter Nilsson wrote:
> On Fri, 14 Nov 2014, Radovan Obradovic wrote:
>> index eb37bfe..ddaf8e0 100644
>> --- a/gcc/toplev.c
>> +++ b/gcc/toplev.c
>
>> @@ -1605,6 +1612,11 @@ process_options (void)
>>     /* Save the current optimization options.  */
>>     optimization_default_node = build_optimization_node (&global_options);
>>     optimization_current_node = optimization_default_node;
>> +
>> +  /* Disable use caller save optimization if profiler is active or port
>> +     does not emit prologue and epilogue as RTL.  */
>> +  if (profile_flag || !HAVE_prologue || !HAVE_epilogue)
>> +    flag_use_caller_save = 0;
>>   }
>
> This seems wrong.  Why disable caller-save regardless of
> profile_flag; is there some long-standing bug in caller-save for
> "old" targets?
It's actually -fipa-ra, use-caller-save was the initial name for the 
option and everyone agreed it was poorly named.

The problem is that with -fipa-ra we analyze the generated RTL to 
determine register usage.  We later use that information to optimize 
register allocation in callers.

That works fine and good except for cases where we're emitting code 
textually rather than via RTL.  That happens with profiling, textual 
prologue or textual epilogue.  Thus the test is correct.

Jeff
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/aru-2.c b/gcc/testsuite/gcc.dg/aru-2.c
new file mode 100644
index 0000000..624bd7f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/aru-2.c
@@ -0,0 +1,27 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -pg" } */
+
+static int __attribute__((noinline))
+bar (int x)
+{
+  return x + 3;
+}
+
+int __attribute__((noinline))
+foo (int y0, int y1, int y2, int y3, int y4)
+{
+  int r = 0;
+  r += bar (r + y4);
+  r += bar (r + y3);
+  r += bar (r + y2);
+  r += bar (r + y1);
+  r += bar (r + y0);
+  return r;
+}
+
+int
+main (void)
+{
+  int z = foo (0, 1, 2, 3, 4);
+  return !(z == 191);
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index eb37bfe..ddaf8e0 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -111,6 +111,13 @@  along with GCC; see the file COPYING3.  If not see
 				   declarations for e.g. AIX 4.x.  */
 #endif
 
+#ifndef HAVE_epilogue
+#define HAVE_epilogue 0
+#endif
+#ifndef HAVE_prologue
+#define HAVE_prologue 0
+#endif
+
 #include <new>
 
 static void general_init (const char *);
@@ -1605,6 +1612,11 @@  process_options (void)
   /* Save the current optimization options.  */
   optimization_default_node = build_optimization_node (&global_options);
   optimization_current_node = optimization_default_node;
+
+  /* Disable use caller save optimization if profiler is active or port
+     does not emit prologue and epilogue as RTL.  */
+  if (profile_flag || !HAVE_prologue || !HAVE_epilogue)
+    flag_use_caller_save = 0;
 }
 
 /* This function can be called multiple times to reinitialize the compiler