[Mesa-dev] [PATCH] configure.ac: Save user {C, CXX}FLAGS and append them at end.

Mon Jan 7 05:14:36 PST 2013

On Mon, Dec 31, 2012 at 6:03 AM, Dan Nicholson <dbn.lists at gmail.com> wrote:
> On Sun, Dec 23, 2012 at 3:41 AM, Johannes Obermayr
> <johannesobermayr at gmx.de> wrote:
>> Am Samstag, 22. Dezember 2012, 16:34:48 schrieben Sie:
>>> On Sat, Dec 22, 2012 at 10:47 AM, Johannes Obermayr
>>> <johannesobermayr at 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
>>> >> <johannesobermayr at 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.

The following two patches provide a short term fix by only adding '-g
-O0' for --enable-debug when CFLAGS/CXXFLAGS have not been set by the
user. I think that's the simplest way to still get the functionality
from -DDEBUG while respecting the user's preferences. It doesn't try
to fix the general issue of configure overriding the user's flags,
though.

--
Dan