On Sun, Dec 23, 2012 at 3:41 AM, Johannes Obermayr <johannesoberm...@gmx.de> wrote: > Am Samstag, 22. Dezember 2012, 16:34:48 schrieben Sie: >> On Sat, Dec 22, 2012 at 10:47 AM, Johannes Obermayr >> <johannesoberm...@gmx.de> wrote: >> > Am Samstag, 22. Dezember 2012, 09:21:33 schrieb Matt Turner: >> >> On Sat, Dec 22, 2012 at 9:16 AM, Johannes Obermayr >> >> <johannesoberm...@gmx.de> wrote: >> >> > This way the user has the privilege of last decision and so the option >> >> > to build an optimized debug build again. >> >> > --- >> >> >> >> You can just do CFLAGS="-g -O2" ./configure can't you? >> > >> > Nope. >> > >> > CXXFLAGS='-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 >> > -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g' >> > CFLAGS='-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector >> > -funwind-tables -fasynchronous-unwind-tables -g' ./autogen.sh >> > --host=x86_64-suse-linux-gnu --build=x86_64-suse-linux-gnu >> > --program-prefix= --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin >> > --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share >> > --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/lib >> > --localstatedir=/var --sharedstatedir=/usr/com --mandir=/usr/share/man >> > --infodir=/usr/share/info --disable-dependency-tracking --enable-xvmc >> > --enable-vdpau --enable-texture-float --enable-debug >> > --with-dri-drivers=i915,i965,nouveau,r200,radeon,swrast >> > --with-gallium-drivers=i915,nouveau,r300,r600,svga,swrast --enable-dri >> > --enable-glx --enable-osmesa --enable-gles1 --enable-gles2 --enable-openvg >> > --enable-shared-glapi --enable-shared-gallium --enable-gbm --enable-xa --ena >> > ble-gallium-egl --enable-gallium-llvm --enable-gallium-gbm >> > --enable-opencl --enable-r600-llvm-compiler --enable-gallium-g3dvl >> > --enable-glx-tls >> > >> > Resulting Makefile: >> > before: >> > CFLAGS = -fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 >> > -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g -Wall >> > -std=c99 -Werror=implicit-function-declaration -Werror=missing-prototypes >> > -fno-strict-aliasing -fno-builtin-memcmp -g -O0 >> > >> > after: >> > CFLAGS = -Wall -std=c99 -Werror=implicit-function-declaration >> > -Werror=missing-prototypes -fno-strict-aliasing -fno-builtin-memcmp -g -O0 >> > -fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector >> > -funwind-tables -fasynchronous-unwind-tables -g >> > >> > You see configure CFLAGS and initial CFLAGS are vice versa. >> >> The real fix here is not to have configure.ac touch CFLAGS at all like >> any reasonable autotools project. That was an unfortunate shortcut >> during the initial work. That would be a lot more effort, though. > > Initial work after ~ 5 years? > http://cgit.freedesktop.org/mesa/mesa/commit/?id=122345876479cf5cf553e38162ab105658614ab7
Not sure what you mean by this. When I wrote configure.ac however many years ago, I didn't cleanly split off CFLAGS set by mesa from the user's CFLAGS as a shortcut. Looking back now, I should have just bit the bullet and made something like MESA_CFLAGS/MESA_LDFLAGS and made all the Makefiles use that. > Btw: > http://cgit.freedesktop.org/mesa/mesa/commit/?id=ff2efdf5997d20b41f7a82b77118366e6fbd23bc > http://cgit.freedesktop.org/mesa/mesa/commit/?id=33ae29c93b8f70a86dcedc495dd658a5d5679db3 > > This patch covers the motivation for commit 33ae29c. > >> >> However, the typical convention is that the user's CFLAGS come first >> (see any automake COMPILE or LTCOMPILE definition), mostly so that any >> user specified -I paths take precedence. That's unfortunate when -O >> options to gcc are last one wins, but that's how things have typically >> been. I would make this change very cautiously. >> >> -- >> Dan > > And here is the real reason for my request: > > /usr/lib64/mesa-demos/egl/opengl/xeglgears > > AMD Fusion (fps) Nvidia ION (fps) > -O0 -O2 -O0 -O2 > 562,7 750,778 0,33 641,851 921,464 0,44 > 701,756 662,877 -0,06 693,752 1000,196 0,44 > 571,566 665,052 0,16 692,196 936,147 0,35 > 542,715 664,658 0,22 693,733 995,499 0,43 > 542,935 664,78 0,22 692,702 992,634 0,43 > 542,574 665,398 0,23 692,835 998,665 0,44 > 542,945 663,087 0,22 691,353 998,319 0,44 > 541,732 664,253 0,23 656,32 995,91 0,52 > 543,392 665,078 0,22 691,084 994,037 0,44 > 542,901 663,406 0,22 693,464 996,858 0,44 > 5635,216 6729,367 0,19 6839,29 9829,729 0,44 > (sum) > > -> Performance + 19 % on AMD Fusion, + 44 % on Nvidia ION on debug build with > -O2. :-) > > Usual users should build Mesa with default FLAGS. Advanced users should be > able to build it with their own FLAGS without modifying source. Right, --enable-debug is forcing you to have -O0. That's the problem. Mesa should not be overriding user defined flags such as optimization unless it's necessary to get Mesa to build. There are two issues here: 1. Adding -O0 in --enable-debug is a convenience, but as you see it can conflicts with the user's flags. This is why I left -O out of --enable-debug from the beginning. http://cgit.freedesktop.org/mesa/mesa/commit/?id=23656c47 That said, the whole option is just a helper. If you know what you're doing, you can easily build with CFLAGS='-g -O2 -DDEBUG' and have all the control you want. --enable-32-bit/--enable-64-bit are the same way. They're convenience options I added that fiddle with your CFLAGS so you don't have to, but they clearly have to make compromises about "the right thing". All three options should have their help text updated to indicate that they're just passing flags to the compiler and not doing anything special to the code. That way people that know what they're doing can set things up the way they want. An alternate fix would be to check if CFLAGS contains -O already and leave out -O0 in that case. 2. More generally, Mesa should be not be touching the user's CFLAGS at all. All the flags Mesa needs to pass to the compiler/linker should be split off into separate variables so the user can be free to do whatever they want with CFLAGS/CPPFLAGS/LDFLAGS/whatever. Look at any cleanly constructed autotools project and you will see it does not touch the user's environment variables at all. And, in fact, automake/libtool does exactly what you're requesting by putting CFLAGS/LDFLAG at the end of the command. From one of my other projects: COMPILE = $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) \ $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) LTCOMPILE = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \ $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) \ $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) \ $(AM_CFLAGS) $(CFLAGS) LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \ $(LIBTOOLFLAGS) --mode=link $(CCLD) $(AM_CFLAGS) $(CFLAGS) \ $(AM_LDFLAGS) $(LDFLAGS) -o $@ I'm pretty sure the right way to fix this bug is to drop the whole USER_*FLAGS thing, bite the bullet, and spray MESA_CFLAGS/MESA_CPPFLAGS/MESA_LDFLAGS all over the tree. Or cheat and AC_SUBST AM_CFLAGS etc. from configure.ac and check that no one overrides it in Makefile.am. Looking back at my previous email, I was wrong that automake/libtool put CFLAGS at the beginning of the command line. This patch has a glaring bug in it, though. By setting CFLAGS/CXXFLAGS to "" during configure, you lose the user's settings while doing the feature tests. This is bad since then the feature tests have been performed with different compiler settings the rest of the build. It's trivial to think up ways (like missing -I while doing AC_CHECK_HEADER) that the build could break in subtle ways. I'm afraid this issue needs a more invasive fix. Either that larger fix I discussed above, or just make sure that CFLAGS/CXXFLAGS are always prepended to in configure as a short term fix. -- Dan _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
