[PATCH] clean up CFLAGS

[PATCH] clean up CFLAGS

[Charles Coffing]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

This patch cleans up the usage of CFLAGS.  This is nice for packagers,
who would like to control the base compilation flags from a central
place.

I've put some default CFLAGS (-Wall -O3) in Config.mk, and removed
redundant flags from the Makefiles.  I've also fixed a few places that
didn't properly inherit CFLAGS from Config.mk (but not vmxassist or
hvmloader; those ignore global defaults like before).  Also, individual
makefiles still add their own custom flags (-g, -Werror, -f*, etc) just
as before.

Please apply to xen-unstable.

Signed-off-by: Charles Coffing <ccoffing@novell.com>

Thanks,
Charles
 

[-- Attachment #2: xen-cflags.diff --]
[-- Type: application/octet-stream, Size: 13199 bytes --]

Index: xen-unstable/tools/misc/miniterm/Makefile
===================================================================
--- xen-unstable.orig/tools/misc/miniterm/Makefile
+++ xen-unstable/tools/misc/miniterm/Makefile
@@ -1,9 +1,10 @@
+XEN_ROOT:=../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
 INSTALL		= install
 INSTALL_PROG	= $(INSTALL) -m0755
 INSTALL_DIR	= $(INSTALL) -d -m0755
 
-CC     = gcc
-CFLAGS = -Wall -O3
 TARGET = miniterm
 
 all: $(TARGET)
Index: xen-unstable/tools/ioemu/Makefile
===================================================================
--- xen-unstable.orig/tools/ioemu/Makefile
+++ xen-unstable/tools/ioemu/Makefile
@@ -3,7 +3,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 
 -include config-host.mak
 
-CFLAGS+=-Wall -O2 -g -fno-strict-aliasing 
+CFLAGS+= -g -fno-strict-aliasing
 ifdef CONFIG_DARWIN
 CFLAGS+= -mdynamic-no-pic
 endif
Index: xen-unstable/tools/ioemu/target-i386-dm/Makefile
===================================================================
--- xen-unstable.orig/tools/ioemu/target-i386-dm/Makefile
+++ xen-unstable/tools/ioemu/target-i386-dm/Makefile
@@ -13,7 +13,7 @@ ifdef CONFIG_USER_ONLY
 VPATH+=:$(SRC_PATH)/linux-user
 DEFINES+=-I$(SRC_PATH)/linux-user -I$(SRC_PATH)/linux-user/$(TARGET_ARCH)
 endif
-CFLAGS+=-Wall -O2 -g -fno-strict-aliasing
+CFLAGS+=-g -fno-strict-aliasing
 LDFLAGS=-g
 LIBS=
 HELPER_CFLAGS=$(CFLAGS)
Index: xen-unstable/Config.mk
===================================================================
--- xen-unstable.orig/Config.mk
+++ xen-unstable/Config.mk
@@ -20,6 +20,7 @@ STRIP      = $(CROSS_COMPILE)strip
 OBJCOPY    = $(CROSS_COMPILE)objcopy
 OBJDUMP    = $(CROSS_COMPILE)objdump
 
+CFLAGS      ?= -Wall -O3
 DISTDIR     ?= $(XEN_ROOT)/dist
 
 INSTALL      = install
Index: xen-unstable/tools/console/Makefile
===================================================================
--- xen-unstable.orig/tools/console/Makefile
+++ xen-unstable/tools/console/Makefile
@@ -9,8 +9,7 @@ INSTALL         = install
 INSTALL_PROG    = $(INSTALL) -m0755
 INSTALL_DIR     = $(INSTALL) -d -m0755
 
-CFLAGS  += -Wall -Werror -g3
-
+CFLAGS  += -Werror
 CFLAGS  += -I $(XEN_LIBXC)
 CFLAGS  += -I $(XEN_XENSTORE)
 
Index: xen-unstable/tools/misc/Makefile
===================================================================
--- xen-unstable.orig/tools/misc/Makefile
+++ xen-unstable/tools/misc/Makefile
@@ -5,7 +5,7 @@ INSTALL_DIR	= $(INSTALL) -d -m0755
 XEN_ROOT=../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS   += -Wall -Werror -O3 
+CFLAGS   += -Werror
 
 INCLUDES += -I $(XEN_XC)
 INCLUDES += -I $(XEN_LIBXC)
Index: xen-unstable/tools/xenmon/Makefile
===================================================================
--- xen-unstable.orig/tools/xenmon/Makefile
+++ xen-unstable/tools/xenmon/Makefile
@@ -22,7 +22,7 @@ SBINDIR = $(PREFIX)/sbin
 XEN_ROOT=../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS += -Wall -Werror -g
+CFLAGS  += -Werror -g
 CFLAGS  += -I $(XEN_XC)
 CFLAGS  += -I $(XEN_LIBXC)
 LDFLAGS += -L $(XEN_LIBXC)
Index: xen-unstable/tools/blktap/Makefile
===================================================================
--- xen-unstable.orig/tools/blktap/Makefile
+++ xen-unstable/tools/blktap/Makefile
@@ -22,10 +22,8 @@ LIBS     := -lpthread -lz
 SRCS     :=
 SRCS     += blktaplib.c xenbus.c blkif.c
 
-CFLAGS   += -Wall
 CFLAGS   += -Werror
 CFLAGS   += -Wno-unused
-#CFLAGS   += -O3
 CFLAGS   += -g3
 CFLAGS   += -fno-strict-aliasing
 CFLAGS   += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
Index: xen-unstable/tools/blktap/parallax/Makefile
===================================================================
--- xen-unstable.orig/tools/blktap/parallax/Makefile
+++ xen-unstable/tools/blktap/parallax/Makefile
@@ -31,10 +31,8 @@ VDI_TOOLS += vdi_fill
 VDI_TOOLS += vdi_tree
 VDI_TOOLS += vdi_validate
 
-CFLAGS   += -Wall
 CFLAGS   += -Werror
 CFLAGS   += -Wno-unused
-#CFLAGS   += -O3
 CFLAGS   += -g3
 CFLAGS   += -fno-strict-aliasing
 CFLAGS   += $(INCLUDES)
@@ -58,7 +56,7 @@ parallax: $(PLX_SRCS)
 	$(CC) $(CFLAGS) -o parallax -L.. $(LDFLAGS) $(PLX_SRCS)
 
 ${VDI_TOOLS}: %: %.c $(VDI_SRCS)
-	$(CC) $(CFLAGS) -g3 -o $@ $@.c $(LDFLAGS) $(VDI_SRCS)
+	$(CC) $(CFLAGS) -o $@ $@.c $(LDFLAGS) $(VDI_SRCS)
 
 .PHONY: TAGS clean install rpm
--include $(DEPS)
\ No newline at end of file
+-include $(DEPS)
Index: xen-unstable/tools/blktap/ublkback/Makefile
===================================================================
--- xen-unstable.orig/tools/blktap/ublkback/Makefile
+++ xen-unstable/tools/blktap/ublkback/Makefile
@@ -9,10 +9,8 @@ INSTALL_PROG = $(INSTALL) -m0755
 IBIN         = ublkback
 INSTALL_DIR  = /usr/sbin
 
-CFLAGS   += -Wall
 CFLAGS   += -Werror
 CFLAGS   += -Wno-unused
-#CFLAGS   += -O3
 CFLAGS   += -g3
 CFLAGS   += -fno-strict-aliasing
 CFLAGS   += -I $(XEN_LIBXC)
Index: xen-unstable/tools/debugger/pdb/Makefile
===================================================================
--- xen-unstable.orig/tools/debugger/pdb/Makefile
+++ xen-unstable/tools/debugger/pdb/Makefile
@@ -20,7 +20,6 @@ INCLUDES   += -I ./linux-2.6-module
 INCLUDES   += -I $(OCAML_ROOT)/lib/ocaml
 
 CFLAGS     += $(INCLUDES)
-CFLAGS     += -Wall
 CFLAGS     += -Werror
 CFLAGS     += -g
 
Index: xen-unstable/tools/vnet/libxutil/Makefile
===================================================================
--- xen-unstable.orig/tools/vnet/libxutil/Makefile
+++ xen-unstable/tools/vnet/libxutil/Makefile
@@ -29,8 +29,7 @@ LIB_SRCS += util.c
 LIB_OBJS := $(LIB_SRCS:.c=.o)
 PIC_OBJS := $(LIB_SRCS:.c=.opic)
 
-CFLAGS   += -Wall -Werror -O3 -fno-strict-aliasing
-CFLAGS   += -g
+CFLAGS   += -g -Werror -fno-strict-aliasing
 
 # Get gcc to generate the dependencies for us.
 CFLAGS   += -Wp,-MD,.$(@F).d
Index: xen-unstable/tools/vnet/vnetd/Makefile
===================================================================
--- xen-unstable.orig/tools/vnet/vnetd/Makefile
+++ xen-unstable/tools/vnet/vnetd/Makefile
@@ -43,8 +43,6 @@ CPPFLAGS += -D __ARCH_I386_ATOMIC__
 
 #----------------------------------------------------------------------------
 CFLAGS += -g
-CFLAGS += -O2
-CFLAGS += -Wall
 CFLAGS += $(INCLUDES) $(LIBS)
 
 LDFLAGS += $(LIBS)
Index: xen-unstable/tools/misc/cpuperf/Makefile
===================================================================
--- xen-unstable.orig/tools/misc/cpuperf/Makefile
+++ xen-unstable/tools/misc/cpuperf/Makefile
@@ -17,8 +17,6 @@ INSTALL_DIR	= $(INSTALL) -d -m0755
 XEN_ROOT=../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS      += -Wall -O3 
-
 HDRS         = $(wildcard *.h)
 SRCS         = $(wildcard *.c)
 OBJS         = $(patsubst %.c,%.o,$(SRCS))
Index: xen-unstable/xen/arch/x86/Rules.mk
===================================================================
--- xen-unstable.orig/xen/arch/x86/Rules.mk
+++ xen-unstable/xen/arch/x86/Rules.mk
@@ -15,7 +15,7 @@ CFLAGS  += -I$(BASEDIR)/include/asm-x86/
 CFLAGS  += -I$(BASEDIR)/include/asm-x86/mach-default
 
 ifneq ($(debug),y)
-CFLAGS  += -O3 -fomit-frame-pointer
+CFLAGS  += -fomit-frame-pointer
 endif
 
 # Prevent floating-point variables from creeping into Xen.
Index: xen-unstable/tools/misc/mbootpack/Makefile
===================================================================
--- xen-unstable.orig/tools/misc/mbootpack/Makefile
+++ xen-unstable/tools/misc/mbootpack/Makefile
@@ -20,10 +20,8 @@ GDB	:= gdb
 INCS	:= -I. -I-
 DEFS	:= 
 LDFLAGS	:= 
-CFLAGS	:= -Wall -Wpointer-arith -Wcast-qual -Wno-unused -Wno-format
-CFLAGS	+= -Wmissing-prototypes
-#CFLAGS	+= -pipe -g -O0 -Wcast-align
-CFLAGS	+= -pipe -O3 
+CFLAGS	+= -Wpointer-arith -Wcast-qual -Wno-unused -Wno-format
+CFLAGS	+= -Wmissing-prototypes -pipe
 
 #  What object files need building for the program
 OBJS	:= mbootpack.o buildimage.o
Index: xen-unstable/tools/debugger/libxendebug/Makefile
===================================================================
--- xen-unstable.orig/tools/debugger/libxendebug/Makefile
+++ xen-unstable/tools/debugger/libxendebug/Makefile
@@ -14,7 +14,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 
 SRCS     := xendebug.c
 
-CFLAGS   += -Wall -Werror -O3 -fno-strict-aliasing
+CFLAGS   += -Werror -fno-strict-aliasing
 CFLAGS   += $(INCLUDES) -I. -I$(XEN_ROOT)/tools/libxc
 # Get gcc to generate the dependencies for us.
 CFLAGS   += -Wp,-MD,.$(@F).d
Index: xen-unstable/tools/libxc/Makefile
===================================================================
--- xen-unstable.orig/tools/libxc/Makefile
+++ xen-unstable/tools/libxc/Makefile
@@ -48,9 +48,7 @@ BUILD_SRCS += xc_linux_save.c
 BUILD_SRCS += xc_hvm_build.c
 endif
 
-CFLAGS   += -Wall
 CFLAGS   += -Werror
-CFLAGS   += -O3
 CFLAGS   += -fno-strict-aliasing
 CFLAGS   += $(INCLUDES) -I.
 
Index: xen-unstable/tools/security/Makefile
===================================================================
--- xen-unstable.orig/tools/security/Makefile
+++ xen-unstable/tools/security/Makefile
@@ -1,9 +1,7 @@
 XEN_ROOT = ../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS   += -Wall
 CFLAGS   += -Werror
-CFLAGS   += -O3
 CFLAGS   += -fno-strict-aliasing
 CFLAGS   += -I.
 
Index: xen-unstable/tools/vtpm/Rules.mk
===================================================================
--- xen-unstable.orig/tools/vtpm/Rules.mk
+++ xen-unstable/tools/vtpm/Rules.mk
@@ -14,7 +14,7 @@ INSTALL_DIR     = $(INSTALL) -d -m0755
 TOOLS_INSTALL_DIR = $(DESTDIR)/usr/bin
 
 # General compiler flags
-CFLAGS   = -Wall -Werror -g3 -I.
+CFLAGS  += -Werror -I.
 
 # For generating dependencies
 CFLAGS	+= -Wp,-MD,.$(@F).d
Index: xen-unstable/tools/xcutils/Makefile
===================================================================
--- xen-unstable.orig/tools/xcutils/Makefile
+++ xen-unstable/tools/xcutils/Makefile
@@ -19,7 +19,7 @@ PROGRAMS_INSTALL_DIR = /usr/$(LIBDIR)/xe
 
 INCLUDES += -I $(XEN_LIBXC)
 
-CFLAGS += -Wall -Werror -O3 -fno-strict-aliasing
+CFLAGS += -Werror -fno-strict-aliasing
 CFLAGS += $(INCLUDES)
 
 # Make gcc generate dependencies.
Index: xen-unstable/tools/xentrace/Makefile
===================================================================
--- xen-unstable.orig/tools/xentrace/Makefile
+++ xen-unstable/tools/xentrace/Makefile
@@ -6,7 +6,7 @@ INSTALL_DATA	= $(INSTALL) -m0644
 XEN_ROOT=../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS  += -Wall -Werror -O3
+CFLAGS  += -Werror
 
 CFLAGS  += -I $(XEN_XC)
 CFLAGS  += -I $(XEN_LIBXC)
Index: xen-unstable/tools/misc/lomount/Makefile
===================================================================
--- xen-unstable.orig/tools/misc/lomount/Makefile
+++ xen-unstable/tools/misc/lomount/Makefile
@@ -6,7 +6,7 @@ INSTALL_DATA	= $(INSTALL) -m0644
 XEN_ROOT=../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS  += -Wall -Werror -O3
+CFLAGS  += -Werror
 
 HDRS     = $(wildcard *.h)
 OBJS     = $(patsubst %.c,%.o,$(wildcard *.c))
Index: xen-unstable/tools/misc/nsplitd/Makefile
===================================================================
--- xen-unstable.orig/tools/misc/nsplitd/Makefile
+++ xen-unstable/tools/misc/nsplitd/Makefile
@@ -1,6 +1,6 @@
+XEN_ROOT:=../..
+include $(XEN_ROOT)/tools/Rules.mk
 
-CC     = gcc
-CFLAGS = -Wall -O3
 CFILES = $(wildcard *.c)
 
 HDRS     = $(wildcard *.h)
Index: xen-unstable/tools/vtpm_manager/Rules.mk
===================================================================
--- xen-unstable.orig/tools/vtpm_manager/Rules.mk
+++ xen-unstable/tools/vtpm_manager/Rules.mk
@@ -14,7 +14,7 @@ INSTALL_DIR     = $(INSTALL) -d -m0755
 TOOLS_INSTALL_DIR = $(DESTDIR)/usr/bin
 
 # General compiler flags
-CFLAGS	= -Wall -Werror -g3 -I.
+CFLAGS	+= -Werror -I.
 
 # For generating dependencies
 CFLAGS	+= -Wp,-MD,.$(@F).d
Index: xen-unstable/tools/firmware/hvmloader/Makefile
===================================================================
--- xen-unstable.orig/tools/firmware/hvmloader/Makefile
+++ xen-unstable/tools/firmware/hvmloader/Makefile
@@ -19,7 +19,8 @@
 #
 
 # External CFLAGS can do more harm than good.
-CFLAGS :=
+# Start with a benign set, so I don't get some other defaults.
+CFLAGS := -Wall -O2
 
 XEN_ROOT = ../../..
 include $(XEN_ROOT)/Config.mk
@@ -38,7 +39,7 @@ CFLAGS  += $(call test-gcc-flag,$(CC),-f
 CFLAGS  += $(call test-gcc-flag,$(CC),-fno-stack-protector-all)
 
 OBJCOPY  = objcopy
-CFLAGS  += $(DEFINES) -I. $(XENINC) -Wall -fno-builtin -O2 -msoft-float
+CFLAGS  += $(DEFINES) -I. $(XENINC) -fno-builtin -msoft-float
 CFLAGS  += -m32 -march=i686
 LDFLAGS  = -m32 -nostdlib -Wl,-N -Wl,-Ttext -Wl,$(LOADADDR)
 
Index: xen-unstable/tools/firmware/vmxassist/Makefile
===================================================================
--- xen-unstable.orig/tools/firmware/vmxassist/Makefile
+++ xen-unstable/tools/firmware/vmxassist/Makefile
@@ -19,7 +19,8 @@
 #
 
 # External CFLAGS can do more harm than good.
-CFLAGS :=
+# Start with a benign set, so I don't get some other defaults.
+CFLAGS := -Wall -O2
 
 XEN_ROOT = ../../..
 include $(XEN_ROOT)/Config.mk
@@ -37,7 +38,7 @@ CFLAGS  += $(call test-gcc-flag,$(CC),-f
 
 CPP      = cpp -P
 OBJCOPY  = objcopy -p -O binary -R .note -R .comment -R .bss -S --gap-fill=0
-CFLAGS  += $(DEFINES) -I. $(XENINC) -Wall -fno-builtin -O2 -msoft-float
+CFLAGS  += $(DEFINES) -I. $(XENINC) -fno-builtin -msoft-float
 CFLAGS  += -m32 -march=i686
 LDFLAGS  = -m elf_i386
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] clean up CFLAGS

[Keir Fraser]

On 9 Mar 2006, at 19:42, Charles Coffing wrote:

> I've put some default CFLAGS (-Wall -O3) in Config.mk, and removed
> redundant flags from the Makefiles.  I've also fixed a few places that
> didn't properly inherit CFLAGS from Config.mk (but not vmxassist or
> hvmloader; those ignore global defaults like before).  Also, individual
> makefiles still add their own custom flags (-g, -Werror, -f*, etc) just
> as before.
>
> Please apply to xen-unstable.

I'll take a look at applying some of these piecemeal. After a quick 
browse I see that the miniterm Makefile is broken (it is intended to be 
built and installed on the external box, not the Xen box, so HOSTCC is 
probably right for it but the potentially cross-compiling CC is 
definitely not), and the changes to vmxassist and hvmloader have no 
effect and I see no reason to prefer them over what is there already. 
No comment on the rest as yet...

  -- Keir

Re: [PATCH] clean up CFLAGS

[Charles Coffing]

>>> On Thu, Mar 9, 2006 at 12:47 PM, in message
<7b2382655a2fb46f17db47d57d745413@cl.cam.ac.uk>, Keir Fraser
<Keir.Fraser@cl.cam.ac.uk> wrote: 

> On 9 Mar 2006, at 19:42, Charles Coffing wrote:
> 
>> I've put some default CFLAGS (- Wall - O3) in Config.mk, and removed
>> redundant flags from the Makefiles.  I've also fixed a few places that
>> didn't properly inherit CFLAGS from Config.mk (but not vmxassist or
>> hvmloader; those ignore global defaults like before).  Also, individual
>> makefiles still add their own custom flags (- g, - Werror, - f*, etc) just
>> as before.
>>
>> Please apply to xen- unstable.
> 
> I'll take a look at applying some of these piecemeal. After a quick 
> browse I see that the miniterm Makefile is broken (it is intended to be 
> built and installed on the external box, not the Xen box, so HOSTCC is 
> probably right for it but the potentially cross- compiling CC is 
> definitely not), and the changes to vmxassist and hvmloader have no 
> effect and I see no reason to prefer them over what is there already. 
> No comment on the rest as yet...

Ah, agreed on both counts.

Regarding the change in vmxassist and hvmloader:  I was being overly cautious, setting CFLAGS to a non-empty string to be sure it wasn't defaulted to something else, but indeed setting to an empty string as was done before is sufficient.  So I have thrown out the changes to both of those Makefiles.

nsplitd is to be treated the same as miniterm, correct?  So you should both of their respective patches.  (When packaging, we of course would build those things with the same flags, and for the same architecture, but that is a patch I can carry separately .)

Re: [PATCH] clean up CFLAGS

[Keir Fraser]

On 9 Mar 2006, at 20:16, Charles Coffing wrote:

> Ah, agreed on both counts.
>
> Regarding the change in vmxassist and hvmloader:  I was being overly 
> cautious, setting CFLAGS to a non-empty string to be sure it wasn't 
> defaulted to something else, but indeed setting to an empty string as 
> was done before is sufficient.  So I have thrown out the changes to 
> both of those Makefiles.
>
> nsplitd is to be treated the same as miniterm, correct?  So you should 
> both of their respective patches.  (When packaging, we of course would 
> build those things with the same flags, and for the same architecture, 
> but that is a patch I can carry separately .)

I've also now pulled the debug option out into Config.mk, and therefore 
pulled -O3 and -fomit-frame-pointer into that same file. I also fixed 
up the ioemu build to respect CFLAGS (was being overridden by a file 
generated by the configure script) -- in general I think the build 
process for ioemu needs better integration with our build system.

  -- Keir

Re: [PATCH] clean up CFLAGS

[Keir Fraser]

On 9 Mar 2006, at 19:47, Keir Fraser wrote:

> I'll take a look at applying some of these piecemeal. After a quick 
> browse I see that the miniterm Makefile is broken (it is intended to 
> be built and installed on the external box, not the Xen box, so HOSTCC 
> is probably right for it but the potentially cross-compiling CC is 
> definitely not), and the changes to vmxassist and hvmloader have no 
> effect and I see no reason to prefer them over what is there already. 
> No comment on the rest as yet...

The rest looked okay to me, except I'm not 100% sure about pulling the 
optimisation level out to Config.mk. It means if things liek Xen itself 
want to make a debug build, they'd have to strip out the passed-in 
'-O?' option which is a pain. So I left that part of the patch out for 
now. And I made -Wall unconditionally added to CFLAGS in Config.mk 
instead of "?=". I don't think it can ever hurt?

  -- Keir

Re: [PATCH] clean up CFLAGS

[Andi Kleen]

"Charles Coffing" <ccoffing@novell.com> writes:

> This patch cleans up the usage of CFLAGS.  This is nice for packagers,
> who would like to control the base compilation flags from a central
> place.
> 
> I've put some default CFLAGS (-Wall -O3) 

Are you sure -O3 is a good idea? In my experience it just bloats 
.text for most projects because of its aggressive inlining. I would
use -O2.

-Andi

Re: Re: [PATCH] clean up CFLAGS

[Keir Fraser]

On 10 Mar 2006, at 14:50, Andi Kleen wrote:

>> I've put some default CFLAGS (-Wall -O3)
>
> Are you sure -O3 is a good idea? In my experience it just bloats
> .text for most projects because of its aggressive inlining. I would
> use -O2.

That's a good question. All that patch really did was move the 
optimisation default into Config.mk, and the general choice throughout 
the Xen tree was -O3. But it isn't clear that's best.

  -- Keir

Re: Re: [PATCH] clean up CFLAGS

[Andi Kleen]

On Friday 10 March 2006 15:58, Keir Fraser wrote:
> 
> On 10 Mar 2006, at 14:50, Andi Kleen wrote:
> 
> >> I've put some default CFLAGS (-Wall -O3)
> >
> > Are you sure -O3 is a good idea? In my experience it just bloats
> > .text for most projects because of its aggressive inlining. I would
> > use -O2.
> 
> That's a good question. All that patch really did was move the 
> optimisation default into Config.mk, and the general choice throughout 
> the Xen tree was -O3. But it isn't clear that's best.

The main linux kernel switched to -Os by default now. Rationale
is that for kernel code it's best to be as small in icache as possible
and the optimizations in -O2 mostly help loops etc which kernel
code doesn't tend to do a whole lot of. 

Drawback is that it used to trigger some gcc code generation books,
but at least now it seems to be ok (i'm not aware of any current issues)

On x86-64 I'm also using -fno-reorder-blocks. The main reason is that
it also makes the code a bit smaller and more important it's much easier
to read the assembly code during debugging because it matches the C code 
better.

-Andi

Re: Re: [PATCH] clean up CFLAGS

[Keir Fraser]

On 10 Mar 2006, at 08:30, Andi Kleen wrote:

> The main linux kernel switched to -Os by default now. Rationale
> is that for kernel code it's best to be as small in icache as possible
> and the optimizations in -O2 mostly help loops etc which kernel
> code doesn't tend to do a whole lot of.

Well, moving to -O2 at least looks sane. I noticed -O3 can go a bit mad 
with auto inlining.

> On x86-64 I'm also using -fno-reorder-blocks. The main reason is that
> it also makes the code a bit smaller and more important it's much 
> easier
> to read the assembly code during debugging because it matches the C 
> code
> better.

I use that flag too, mainly because I stole x86/64 Xen CFLAGS from 
Linux. :-)

  -- Keir

Re: [PATCH] clean up CFLAGS

[Charles Coffing]

-O2 actually tends to be my preference too for the same reason, but I
was just rearranging the existing options.

I've certainly seen cases (prior to Xen, though) where -O3 was slower
than -O2, probably due to cache effects.


>>> On Fri, Mar 10, 2006 at  7:50 AM, in message
<p73slpquzfr.fsf@verdi.suse.de>,
Andi Kleen <ak@suse.de> wrote: 
> "Charles Coffing" <ccoffing@novell.com> writes:
> 
>> This patch cleans up the usage of CFLAGS.  This is nice for
packagers,
>> who would like to control the base compilation flags from a central
>> place.
>> 
>> I've put some default CFLAGS (- Wall - O3) 
> 
> Are you sure - O3 is a good idea? In my experience it just bloats 
> .text for most projects because of its aggressive inlining. I would
> use - O2.
> 
> - Andi