| Message ID | B580B8E56670DB4993CF59CB657624A84D610631@BADAG02.ba.imgtec.org |
|---|---|
| State | New |
| Headers | show |
On 11/13/14 07:37, Radovan Obradovic wrote: > > 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. Presumably we can get the same kinds of problems with ports that don't emit prologues/epilogues as RTL? 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. Is there any way you could cobble together a testcase for the problem you saw with the kernel? It's OK if the test is target specific, though obviously we get better coverage if it's a target independent test. Your call on that decision. Please repost after updating to test HAVE_prologue and HAVE_epilogue and adding a testcase. Thanks, Jeff
On Nov 13, 2014, at 9:21 AM, Jeff Law <law@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.
On 11/13/14 15:14, Mike Stump wrote: > On Nov 13, 2014, at 9:21 AM, Jeff Law <law@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
On Nov 13, 2014, at 2:33 PM, Jeff Law <law@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.
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
On Nov 14, 2014, at 9:30 AM, Jeff Law <law@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?
On Nov 14, 2014, at 9:51 AM, Mike Stump <mikestump@comcast.net> wrote: > On Nov 14, 2014, at 9:30 AM, Jeff Law <law@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.
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