| Message ID | 1354033263-32748-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/27/2012 05:21 PM, Paolo Bonzini wrote: > Some versions of GCC require insane (>2GB) amounts of memory to compile > translate.o. As a countermeasure, disable the culprit optimization pass. > This should fix the buildbot failure for default_x86_64_fedora16. > Anyway is a good thing to do because people will try to compile 1.3 with > less than 2GB of memory and complain. > > Signed-off-by: Paolo Bonzini<pbonzini@redhat.com> > --- > Makefile.target | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Makefile.target b/Makefile.target > index 8b658c0..d38bb58 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -143,6 +143,12 @@ GENERATED_HEADERS += hmp-commands.h qmp-commands-old.h > > endif # CONFIG_SOFTMMU > > +# Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC > +# and large functions that use global variables. The bug is in all > +# releases of GCC, but it became particularly acute in 4.7.x. We > +# should be able to delete this at the end of 2013. Can we add a version check for gcc here? Alex > +%/translate.o: QEMU_CFLAGS += -fno-gcse > + > nested-vars += obj-y > > # This resolves all nested paths, so it must come last
Il 27/11/2012 17:24, Alexander Graf ha scritto: >> >> +# Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC >> +# and large functions that use global variables. The bug is in all >> +# releases of GCC, but it became particularly acute in 4.7.x. We >> +# should be able to delete this at the end of 2013. > > Can we add a version check for gcc here? I don't think it is useful unless somebody finds that the patch gives substantially worse TCG performance. Paolo
Am 27.11.2012 17:30, schrieb Paolo Bonzini: > Il 27/11/2012 17:24, Alexander Graf ha scritto: >>> +# Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC >>> +# and large functions that use global variables. The bug is in all >>> +# releases of GCC, but it became particularly acute in 4.7.x. We >>> +# should be able to delete this at the end of 2013. >> Can we add a version check for gcc here? > I don't think it is useful unless somebody finds that the patch gives > substantially worse TCG performance. > > Paolo > Hi Paolo, latest native MinGW-w64 uses gcc 4.7.2 for w64. It compiles */translate.c without needing too much RAM. In a short test, I compiled target-ppc/translate.c without and with -fno-gcse. This compiler option increases compilation speed a little and creates a smaller binary (tested with Debian amd64-mingw32msvc-gcc 4.4.4): standard time: user 0m31.966s size: 1113760 70216 1376 1185352 121648 target-ppc/translate.o with -fno-gcse time: user 0m30.542s size: 1111056 70216 1376 1182648 120bb8 target-ppc/translate.o To summarize, -fno-gcse is not needed for MinGW, but I don't expect that it would do any harm there. A real problem could arise from compilers which don't support -fno-gcse. As this option is not checked for compatibility in configure, such compilers would no longer work with unmodified QEMU sources. clang obviously supports -fno-gcse, so maybe we don't have a real problem currently. For the buildbot machines, "configure --enable-debug" wouldsolve the OOM problem, dramatically reduce compilation time, add some compile time checks for TCG, reduce CO2 emission, ... For most buildbots, --enable-debug would be a good choice. There are some kinds of errors which compilers only detect during their optimization pass, so some buildbots should still run without --enable-debug. Do we need -fno-gcse for all */translate.c or only for some of them? The problem with gcc using large quantities of RAM for those files is not new (it was a good RAM tester on a defective PC some time ago for me). I think it is caused by huge switch statements in those files. Splitting those switch statements might also help. If the memory needed grows with n * n (n = number of case statements in one switch statement), then splitting a switch statement in two would reduce the memory needed from 2 GiB to 0.5 GiB. Regards Stefan
On Tue, Nov 27, 2012 at 05:21:03PM +0100, Paolo Bonzini wrote: > Some versions of GCC require insane (>2GB) amounts of memory to compile > translate.o. As a countermeasure, disable the culprit optimization pass. > This should fix the buildbot failure for default_x86_64_fedora16. > Anyway is a good thing to do because people will try to compile 1.3 with > less than 2GB of memory and complain. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > Makefile.target | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Makefile.target b/Makefile.target > index 8b658c0..d38bb58 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -143,6 +143,12 @@ GENERATED_HEADERS += hmp-commands.h qmp-commands-old.h > > endif # CONFIG_SOFTMMU > > +# Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC > +# and large functions that use global variables. The bug is in all > +# releases of GCC, but it became particularly acute in 4.7.x. We > +# should be able to delete this at the end of 2013. > +%/translate.o: QEMU_CFLAGS += -fno-gcse > + > nested-vars += obj-y > > # This resolves all nested paths, so it must come last No objection here. But will we remove this option when GCC fix this pr or we just leave it there? If we're going to remove it in the future, better keep a note on the release change log or somewhere else. Regards, chenwj
于 2012-11-28 10:01, 陳韋任 (Wei-Ren Chen) 写道: > On Tue, Nov 27, 2012 at 05:21:03PM +0100, Paolo Bonzini wrote: >> Some versions of GCC require insane (>2GB) amounts of memory to compile >> translate.o. As a countermeasure, disable the culprit optimization pass. >> This should fix the buildbot failure for default_x86_64_fedora16. >> Anyway is a good thing to do because people will try to compile 1.3 with >> less than 2GB of memory and complain. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> Makefile.target | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Makefile.target b/Makefile.target >> index 8b658c0..d38bb58 100644 >> --- a/Makefile.target >> +++ b/Makefile.target >> @@ -143,6 +143,12 @@ GENERATED_HEADERS += hmp-commands.h qmp-commands-old.h >> >> endif # CONFIG_SOFTMMU >> >> +# Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC >> +# and large functions that use global variables. The bug is in all >> +# releases of GCC, but it became particularly acute in 4.7.x. We >> +# should be able to delete this at the end of 2013. >> +%/translate.o: QEMU_CFLAGS += -fno-gcse >> + >> nested-vars += obj-y >> >> # This resolves all nested paths, so it must come last > > No objection here. But will we remove this option when GCC fix this pr > or we just leave it there? If we're going to remove it in the future, > better keep a note on the release change log or somewhere else. > > Regards, > chenwj > +1, I think -fno-gcse is good enough to fix the problem quickly, and a note would be nice which remind people that translate.o was compiled with special flag now. If some one find it problem with special compiler, or some thing about performance, he can find the reason quickly.
Il 27/11/2012 19:17, Stefan Weil ha scritto: > A real problem could arise from compilers which don't support -fno-gcse. It was introduced in GCC 3.0. > As this option is not checked for compatibility in configure, such > compilers would no longer work with unmodified QEMU sources. clang > obviously supports -fno-gcse, so maybe we don't have a real problem > currently. Yes. > For the buildbot machines, "configure --enable-debug" wouldsolve the > OOM problem, dramatically reduce compilation time, add some > compile time checks for TCG, reduce CO2 emission, ... For most buildbots, > --enable-debug would be a good choice. There are some kinds of errors > which compilers only detect during their optimization pass, so some > buildbots should still run without --enable-debug. No, --enable-debug is not a solution. Fixing GCC bugs, or working around them if possible/useful, is. > Do we need -fno-gcse for all */translate.c or only for some of them? Intel is an order of magnitude worse than the others; however, all of the translate.c are potentially susceptible to this problem. It happens when you have -fPIE or -fPIC, and largish functions that access a lot of globals. translate.c tends to use tcg_ctx, and to inline almost everything into disas_insn... hence the problem. Intel is the worst, but SPARC also requires 300MB for GCSE. PPC is special: it "only" needs 55MB for GCSE, but 150MB for inlining and similarly for other passes---more than other targets. I put "only" in quotes because even Intel with a patched GCC requires only 1.5MB for GCSE, and without sacrificing any optimization. > I think it is caused by huge switch statements > in those files. GCC can handle much worse control flow. Over the years, the developers got really fiendish testcases, mostly template-heavy C++ code or computer-generated. These testcases have a single huge program in a single function, and are "interesting" to say the least. In this case the memory needed is indeed quadratic, but (roughly) in the number of globals that are accessed in the function. GCC uses a garbage collector, but it runs it only between optimization passes in general; usually it doesn't find that much garbage. In this case, GCSE produces hundreds of MB of garbage. Fixing the bug is just a matter of moving some invariant stuff out of an inner loop (interestingly it doesn't save much computation time, only memory). Paolo > Splitting those switch statements might also help. > If the memory needed grows with n * n (n = number of case statements > in one switch statement), then splitting a switch statement in two > would reduce the memory needed from 2 GiB to 0.5 GiB. > > Regards > Stefan > > >
Am 27.11.2012 17:21, schrieb Paolo Bonzini: > Some versions of GCC require insane (>2GB) amounts of memory to compile > translate.o. As a countermeasure, disable the culprit optimization pass. > This should fix the buildbot failure for default_x86_64_fedora16. > Anyway is a good thing to do because people will try to compile 1.3 with > less than 2GB of memory and complain. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Tested-by: Andreas Färber <afaerber@suse.de> Our builds survived the night now, will test v3 next. Andreas
diff --git a/Makefile.target b/Makefile.target index 8b658c0..d38bb58 100644 --- a/Makefile.target +++ b/Makefile.target @@ -143,6 +143,12 @@ GENERATED_HEADERS += hmp-commands.h qmp-commands-old.h endif # CONFIG_SOFTMMU +# Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC +# and large functions that use global variables. The bug is in all +# releases of GCC, but it became particularly acute in 4.7.x. We +# should be able to delete this at the end of 2013. +%/translate.o: QEMU_CFLAGS += -fno-gcse + nested-vars += obj-y # This resolves all nested paths, so it must come last
Some versions of GCC require insane (>2GB) amounts of memory to compile translate.o. As a countermeasure, disable the culprit optimization pass. This should fix the buildbot failure for default_x86_64_fedora16. Anyway is a good thing to do because people will try to compile 1.3 with less than 2GB of memory and complain. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- Makefile.target | 6 ++++++ 1 file changed, 6 insertions(+)