Re: [PATCH] Disable -fuse-caller-save when -pg is active
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
RE: [PATCH] Disable -fuse-caller-save when -pg is active
-Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@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 [l...@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
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On 01/05/15 07:48, Moore, Catherine wrote: -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@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
RE: [PATCH] Disable -fuse-caller-save when -pg is active
-Original Message- From: Jeff Law [mailto:l...@redhat.com] Sent: Monday, January 05, 2015 12:07 PM To: Moore, Catherine; Radovan Obradovic; gcc-patches@gcc.gnu.org Cc: Petar Jovanovic Subject: Re: [PATCH] Disable -fuse-caller-save when -pg is active On 01/05/15 07:48, Moore, Catherine wrote: -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@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. This patch had bit-rotted slightl since the original posting. The -fuse-caller-save option has been changed to -fipa-ra. I committed this updated version of the patch. Catherine Index: toplev.c === --- toplev.c(revision 219188) +++ toplev.c(revision 219208) @@ -113,6 +113,13 @@ 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 *); @@ -1634,6 +1641,11 @@ /* 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_ipa_ra = 0; }
RE: [PATCH] Disable -fuse-caller-save when -pg is active
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
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. Radovan From: Jeff Law [l...@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
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
RE: [PATCH] Disable -fuse-caller-save when -pg is active
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 -
diff --git a/gcc/testsuite/gcc.dg/aru-2.c b/gcc/testsuite/gcc.dg/aru-2.c
new file mode 100644
index 000..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
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On 11/13/14 15:59, Mike Stump wrote:
The problem, since this is a hook now, one can’t just test ifdef
FUNCTION_PROLOGUE.
Right, but we can test the existence of the expander via the HAVE_xxx
interface.
What that can't test is the expander failing (via FAIL;). But the
prologue/epilogue expanders are't on the list of expanders allowed to
use FAIL.
@findex FAIL
@item FAIL
Make the pattern fail on this occasion. When a pattern fails, it means
that the pattern was not truly available. The calling routines in the
compiler will try other strategies for code generation using other patterns.
Failure is currently supported only for binary (addition, multiplication,
shifting, etc.) and bit-field (@code{extv}, @code{extzv}, and @code{insv})
operations.
Jeff
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On Nov 14, 2014, at 9:30 AM, Jeff Law l...@redhat.com wrote: On 11/13/14 15:59, Mike Stump wrote: The problem, since this is a hook now, one can’t just test ifdef FUNCTION_PROLOGUE. Right, but we can test the existence of the expander via the HAVE_xxx interface. Which ones, HAVE_prologue?
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On Nov 14, 2014, at 9:51 AM, Mike Stump mikest...@comcast.net wrote: On Nov 14, 2014, at 9:30 AM, Jeff Law l...@redhat.com wrote: On 11/13/14 15:59, Mike Stump wrote: The problem, since this is a hook now, one can’t just test ifdef FUNCTION_PROLOGUE. Right, but we can test the existence of the expander via the HAVE_xxx interface. Which ones, HAVE_prologue? Ah, ok, I see now given the patch posted what was meant, yeah, that’s fine.
[PATCH] Disable -fuse-caller-save when -pg is active
A problem is detected with building Linux kernel on MIPS platform when both -fuse-caller-save and -pg options are present. The reason for this is that -fuse-caller-save relies on the analysis of RTL code, but when profiling is active (with -pg option) the code is instrumented by adding a call to mcount function at the beginning of each function. And this is realized on the most platforms simply by fprintf function, so this instrumentation is not reflected in RTL code. The result is that bad code is produced. A solution could be to disable -fuse-caller-save when -pg is active. Best regards, Radovan Patch - diff --git a/gcc/toplev.c b/gcc/toplev.c index eb37bfe..010228a 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1605,6 +1605,10 @@ 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. */ + if (profile_flag) +flag_use_caller_save = 0; } /* This function can be called multiple times to reinitialize the compiler
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On Nov 13, 2014, at 9:21 AM, Jeff Law l...@redhat.com wrote: Presumably we can get the same kinds of problems with ports that don't emit prologues/epilogues as RTL? I use prologue/epilogue to emit rtl in my port. I’d like this optimization to kick on in my port, as we do explain everything in rtl. I do what 59 other ports do: (define_expand epilogue [(clobber (const_int 0))] aarch64_expand_epilogue (false); DONE; “ ) this causes HAVE_prologue and HAVE_epilogue to be set. We don’t want to just nix the optimization if HAVE_epilogue or HAVE_prologue is set. This problem is unique to FUNCTION_PROFILER, the interface for it is fprintf (“call mount”). This is what causes the problem. If all those ports used PROFILE_HOOK instead (ia64.c for example), then I don’t think this would be an issue. fprintf bad. FUNCTION_PROFILER is only used when doing profiling. We don’t support fprintf prologues anymore, they were removed years ago. I'm particularly concerned about cases where the prologue or epilogue might use a call-clobbered register as a scratch that isn't used anywhere else in the function. ISTM this test out to be expanded to test HAVE_prologue and HAVE_epilogue as well. So, the solution to that is to look at the registers post prologue/epilogue expansion. Then the rtl has all the bit in it.
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On 11/13/14 15:14, Mike Stump wrote: On Nov 13, 2014, at 9:21 AM, Jeff Law l...@redhat.com wrote: Presumably we can get the same kinds of problems with ports that don't emit prologues/epilogues as RTL? I use prologue/epilogue to emit rtl in my port. I’d like this optimization to kick on in my port, as we do explain everything in rtl. I do what 59 other ports do: (define_expand epilogue [(clobber (const_int 0))] aarch64_expand_epilogue (false); DONE; “ ) this causes HAVE_prologue and HAVE_epilogue to be set. We don’t want to just nix the optimization if HAVE_epilogue or HAVE_prologue is set. No, sorry if I wasn't clear. If we are not emitting prologues or epilogues as RTL, then we want to inhibit because the optimization can't see into what's happening inside the prologue and epilogue. This problem is unique to FUNCTION_PROFILER, the interface for it is fprintf (“call mount”). This is what causes the problem. If all those ports used PROFILE_HOOK instead (ia64.c for example), then I don’t think this would be an issue. fprintf bad. FUNCTION_PROFILER is only used when doing profiling. We don’t support fprintf prologues anymore, they were removed years ago. Did we ever get all the ports converted? jeff
Re: [PATCH] Disable -fuse-caller-save when -pg is active
On Nov 13, 2014, at 2:33 PM, Jeff Law l...@redhat.com wrote: We don’t support fprintf prologues anymore, they were removed years ago. Did we ever get all the ports converted? Ah… sorry, I was wrong. We merely hookized it and the tm.h interface is gone. TARGET_ASM_FUNCTION_PROLOGUE and TARGET_ASM_FUNCTION_EPILOGUE can be used in ways that are opaque to this optimization. 28 ports (or so) still use this method to define prologues. I think we should turn the optimization off when they are used. I had been looking for the pre-hookization and all those remnants are gone. The problem, since this is a hook now, one can’t just test ifdef FUNCTION_PROLOGUE.
