Patchwork Only build with -g CFLAGS/LDFLAGS if using --enable-debug and add --optflags.

login
register
mail settings
Submitter Brad
Date Sept. 6, 2011, 8:02 a.m.
Message ID <20110906080245.GA26732@rox.home.comstyle.com>
Download mbox | patch
Permalink /patch/113502/
State New
Headers show

Comments

Brad - Sept. 6, 2011, 8:02 a.m.
Only build with -g CFLAGS/LDFLAGS if using --enable-debug.
Add --optflags to allow overriding the default optimization
level added to CFLAGS.

This is a first draft of coming up with a patch I could potentially
push upstream based on much cruder local patches to do something
similar. I'm trying to eliminate having to patch the configure
script.

Comments?

---
 configure |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)
Peter Maydell - Sept. 6, 2011, 9:26 a.m.
On 6 September 2011 09:02, Brad <brad@comstyle.com> wrote:
> Only build with -g CFLAGS/LDFLAGS if using --enable-debug.

I'm not convinced -- I think we should always build with -g;
it means you can do at least some debugging on an in-tree
build even if it's the optimised version. Debug info should
be stripped at install time, right?

(I don't think this is a particularly out-on-a-limb view;
it's the standard policy for building things in Debian,
for instance.)

-- PMM
Gerd Hoffmann - Sept. 6, 2011, 10:19 a.m.
On 09/06/11 10:02, Brad wrote:
> Only build with -g CFLAGS/LDFLAGS if using --enable-debug.
> Add --optflags to allow overriding the default optimization
> level added to CFLAGS.
>
> This is a first draft of coming up with a patch I could potentially
> push upstream based on much cruder local patches to do something
> similar. I'm trying to eliminate having to patch the configure
> script.

You don't have to.  You can just run 'make CFLAGS="$optflags"' to 
override the defaults.  Nevertheless having optflags would be nice as 
you don't have to type this for each make run then.

I don't think we should mess with the -g flag.  It should stay enabled 
by default, so you can easily get a useful stacktrace out of a core 
without having to rebuild with debug info first.

cheers,
   Gerd
Peter Maydell - Sept. 6, 2011, 10:35 a.m.
On 6 September 2011 09:02, Brad <brad@comstyle.com> wrote:
> @@ -937,6 +940,7 @@ echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
>  echo "  --cc=CC                  use C compiler CC [$cc]"
>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
>  echo "                           build time"
> +echo "  --optflags=FLAGS         override optimization compiler flags"
>  echo "  --extra-cflags=CFLAGS    append extra C compiler flags QEMU_CFLAGS"
>  echo "  --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS"
>  echo "  --make=MAKE              use specified make [$make]"

No in principle objection to the --optflags bit of the patch, but you
should rearrange things so that the help message can include the
default optflags, something like:
  --optflags=FLAGS    specify compiler flags for optimization [-O2]

(...which might also let you avoid the 'if test -n "$optflags" later.)

-- PMM
Brad - Sept. 6, 2011, 4:30 p.m.
----- Original message -----
> On 09/06/11 10:02, Brad wrote:
> > Only build with -g CFLAGS/LDFLAGS if using --enable-debug.
> > Add --optflags to allow overriding the default optimization
> > level added to CFLAGS.
> > 
> > This is a first draft of coming up with a patch I could potentially
> > push upstream based on much cruder local patches to do something
> > similar. I'm trying to eliminate having to patch the configure
> > script.
> 
> You don't have to.   You can just run 'make CFLAGS="$optflags"' to 
> override the defaults.   Nevertheless having optflags would be nice as 
> you don't have to type this for each make run then.

I do when its unconditionally on the commandline either way. If the configure scipt didnt put it their if CFLAGS wasnt empty it wouldnt be an issue.

> I don't think we should mess with the -g flag.   It should stay enabled 
> by default, so you can easily get a useful stacktrace out of a core 
> without having to rebuild with debug info first.

I dont care what the default is as long as I can disable it without patching.
Juan Quintela - Sept. 7, 2011, 10:54 a.m.
Brad <brad@comstyle.com> wrote:
> ----- Original message -----
>> On 09/06/11 10:02, Brad wrote:
>> > Only build with -g CFLAGS/LDFLAGS if using --enable-debug.
>> > Add --optflags to allow overriding the default optimization
>> > level added to CFLAGS.
>> > 
>> > This is a first draft of coming up with a patch I could potentially
>> > push upstream based on much cruder local patches to do something
>> > similar. I'm trying to eliminate having to patch the configure
>> > script.
>> 
>> You don't have to.   You can just run 'make CFLAGS="$optflags"' to 
>> override the defaults.   Nevertheless having optflags would be nice as 
>> you don't have to type this for each make run then.
>
> I do when its unconditionally on the commandline either way. If the configure scipt didnt put it their if CFLAGS wasnt empty it wouldnt be an issue.

	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")

this is the rule called in rules.make.  QEMU don't use CFLAGS
internally^W^W^W^W, it is only used for -g -O2, so it should be enough
to use make CFLAGS="" or something like that.

>
>> I don't think we should mess with the -g flag.   It should stay enabled 
>> by default, so you can easily get a useful stacktrace out of a core 
>> without having to rebuild with debug info first.
>
> I dont care what the default is as long as I can disable it without patching.

What is the reason for that?  I guess that compilation speed/memory, but
just to be sure.

Later, Juan.
Blue Swirl - Sept. 7, 2011, 7:42 p.m.
On Wed, Sep 7, 2011 at 10:54 AM, Juan Quintela <quintela@redhat.com> wrote:
> Brad <brad@comstyle.com> wrote:
>> ----- Original message -----
>>> On 09/06/11 10:02, Brad wrote:
>>> > Only build with -g CFLAGS/LDFLAGS if using --enable-debug.
>>> > Add --optflags to allow overriding the default optimization
>>> > level added to CFLAGS.
>>> >
>>> > This is a first draft of coming up with a patch I could potentially
>>> > push upstream based on much cruder local patches to do something
>>> > similar. I'm trying to eliminate having to patch the configure
>>> > script.
>>>
>>> You don't have to.   You can just run 'make CFLAGS="$optflags"' to
>>> override the defaults.   Nevertheless having optflags would be nice as
>>> you don't have to type this for each make run then.
>>
>> I do when its unconditionally on the commandline either way. If the configure scipt didnt put it their if CFLAGS wasnt empty it wouldnt be an issue.
>
>        $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
>
> this is the rule called in rules.make.  QEMU don't use CFLAGS
> internally^W^W^W^W, it is only used for -g -O2, so it should be enough
> to use make CFLAGS="" or something like that.
>
>>
>>> I don't think we should mess with the -g flag.   It should stay enabled
>>> by default, so you can easily get a useful stacktrace out of a core
>>> without having to rebuild with debug info first.
>>
>> I dont care what the default is as long as I can disable it without patching.
>
> What is the reason for that?  I guess that compilation speed/memory, but
> just to be sure.

Also disk space. Speed/memory effect seems to be about 3%.

Generating sparc64-softmmu/translate.o with -g, time -v output:
        User time (seconds): 33.00
        System time (seconds): 0.95
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:34.01
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 1636016
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 240264
        Voluntary context switches: 7
        Involuntary context switches: 3201
        Swaps: 0
        File system inputs: 0
        File system outputs: 17488
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0
$ ls -sh translate.o
2.5M translate.o

Without:
        User time (seconds): 31.55
        System time (seconds): 1.10
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:32.73
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 1619056
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 220316
        Voluntary context switches: 7
        Involuntary context switches: 3335
        Swaps: 0
        File system inputs: 0
        File system outputs: 4040
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0
$ ls -sh translate.o
768K translate.o
Stefan Weil - Sept. 7, 2011, 9:31 p.m.
Am 06.09.2011 18:30, schrieb Brad:
> ----- Original message -----
>> On 09/06/11 10:02, Brad wrote:
>>> Only build with -g CFLAGS/LDFLAGS if using --enable-debug.
>>> Add --optflags to allow overriding the default optimization
>>> level added to CFLAGS.
>>>
>>> This is a first draft of coming up with a patch I could potentially
>>> push upstream based on much cruder local patches to do something
>>> similar. I'm trying to eliminate having to patch the configure
>>> script.
>>
>> You don't have to.  You can just run 'make CFLAGS="$optflags"' to
>> override the defaults.  Nevertheless having optflags would be nice as
>> you don't have to type this for each make run then.
>
> I do when its unconditionally on the commandline either way. If the 
> configure scipt didnt put it their if CFLAGS wasnt empty it wouldnt be 
> an issue.
>
>> I don't think we should mess with the -g flag.  It should stay enabled
>> by default, so you can easily get a useful stacktrace out of a core
>> without having to rebuild with debug info first.
>
> I dont care what the default is as long as I can disable it without 
> patching.

I'm sorry but I still don't understand why you want this patch.
It's already possible to run the compiler with or without -g
and with any optimization you want: make CFLAGS="-O1",
make CFLAGS=-g (my favourite), make CFLAGS="-O0", ...

There must be a really good reason for new options like
--optflags. Is this flag some kind of standard which is widely
used (like CFLAGS)? If it isn't, users won't know it, so they
won't use it, so it is useless.

Regards,
Stefan Weil

Patch

diff --git a/configure b/configure
index c3044c7..23a85a7 100755
--- a/configure
+++ b/configure
@@ -77,6 +77,7 @@  path_of() {
 # default parameters
 source_path=`dirname "$0"`
 cpu=""
+optflags=""
 interp_prefix="/usr/gnemul/qemu-%M"
 static="no"
 sparc_cpu=""
@@ -195,6 +196,8 @@  for opt do
   ;;
   --cpu=*) cpu="$optarg"
   ;;
+  --optflags=*) optflags="$optarg"
+  ;;
   --extra-cflags=*) QEMU_CFLAGS="$optarg $QEMU_CFLAGS"
   ;;
   --extra-ldflags=*) LDFLAGS="$optarg $LDFLAGS"
@@ -232,13 +235,11 @@  sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
 
 # default flags for all hosts
 QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS"
-CFLAGS="-g $CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS"
 QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/fpu"
-LDFLAGS="-g $LDFLAGS"
 
 # make source path absolute
 source_path=`cd "$source_path"; pwd`
@@ -518,6 +519,8 @@  for opt do
   ;;
   --cc=*)
   ;;
+  --optflags=*)
+  ;;
   --host-cc=*) host_cc="$optarg"
   ;;
   --make=*) make="$optarg"
@@ -937,6 +940,7 @@  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
 echo "  --cc=CC                  use C compiler CC [$cc]"
 echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
 echo "                           build time"
+echo "  --optflags=FLAGS         override optimization compiler flags"
 echo "  --extra-cflags=CFLAGS    append extra C compiler flags QEMU_CFLAGS"
 echo "  --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS"
 echo "  --make=MAKE              use specified make [$make]"
@@ -2569,8 +2573,15 @@  fi
 # End of CC checks
 # After here, no more $cc or $ld runs
 
-if test "$debug" = "no" ; then
-  CFLAGS="-O2 $CFLAGS"
+if test "$debug" = "yes" ; then
+  CFLAGS="-g $CFLAGS"
+  LDFLAGS="-g $LDFLAGS"
+else
+  if test -n "$optflags" ; then
+      CFLAGS="$optflags $CFLAGS"
+  else
+      CFLAGS="-O2 $CFLAGS"
+  fi
 fi
 
 # Consult white-list to determine whether to enable werror