From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH 2/9] makefile and run_tests tweaks Date: Sat, 28 Dec 2013 22:30:03 -0800 Message-ID: <20131229063003.GA13601@cbox> References: <1386175377-23086-1-git-send-email-drjones@redhat.com> <1386175377-23086-3-git-send-email-drjones@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org To: Andrew Jones Return-path: Received: from mail-pa0-f45.google.com ([209.85.220.45]:55225 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750990Ab3L2G3w (ORCPT ); Sun, 29 Dec 2013 01:29:52 -0500 Received: by mail-pa0-f45.google.com with SMTP id fb1so10607517pad.18 for ; Sat, 28 Dec 2013 22:29:51 -0800 (PST) Content-Disposition: inline In-Reply-To: <1386175377-23086-3-git-send-email-drjones@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Dec 04, 2013 at 05:42:50PM +0100, Andrew Jones wrote: This is a really short commit message, for anyone not super-familiar with the make system in this toolset, it makes it quite hard to understand the change. See some examples below: > - remove a redundant '-display none' > - remove a redundant -g from CFLAGS > - remove a useless -I../include/x86 from CFLAGS why is this include useless? > - remove lib autodep files on make clean why? > > Signed-off-by: Andrew Jones > --- > Makefile | 8 ++++---- > config-x86-common.mak | 16 +++++++--------- > run_tests.sh | 2 +- > 3 files changed, 12 insertions(+), 14 deletions(-) > > diff --git a/Makefile b/Makefile > index 278791dbbef23..697fc2a766966 100644 > --- a/Makefile > +++ b/Makefile > @@ -12,9 +12,9 @@ libgcc := $(shell $(CC) --print-libgcc-file-name) > > libcflat := lib/libcflat.a > cflatobjs := \ > + lib/argv.o \ > lib/printf.o \ > lib/string.o > -cflatobjs += lib/argv.o > > #include architecure specific make rules > include config-$(ARCH).mak > @@ -25,8 +25,8 @@ include config-$(ARCH).mak > cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) > > -CFLAGS += -O1 seems like a good idea to compile test code without optimizations, is that what you're trying to achieve here? Seems to me this should be a separate change or at least motivated in the commit text. > -CFLAGS += $(autodepend-flags) -g -fomit-frame-pointer -Wall I like removing the redundant -g but wouldn't it be cleaner to set the CFLAGS to an empty string in the beginning of the file and then have a seprate line for -g where all the CFLAGS are set? > +CFLAGS += $(autodepend-flags) -Wall > +CFLAGS += $(call cc-option, -fomit-frame-pointer, "") > CFLAGS += $(call cc-option, -fno-stack-protector, "") > CFLAGS += $(call cc-option, -fno-stack-protector-all, "") > CFLAGS += -I. > @@ -51,4 +51,4 @@ install: > install $(tests_and_config) $(DESTDIR) > > clean: arch_clean > - $(RM) *.o *.a .*.d $(libcflat) $(cflatobjs) > + $(RM) *.o *.a .*.d lib/.*.d $(libcflat) $(cflatobjs) > diff --git a/config-x86-common.mak b/config-x86-common.mak > index bf88c672de472..7e481192a0737 100644 > --- a/config-x86-common.mak > +++ b/config-x86-common.mak > @@ -1,13 +1,9 @@ > #This is a make file with common rules for both x86 & x86-64 > > -CFLAGS += -I../include/x86 > - > all: test_cases > > -cflatobjs += \ > - lib/x86/io.o \ > - lib/x86/smp.o > - > +cflatobjs += lib/x86/io.o > +cflatobjs += lib/x86/smp.o > cflatobjs += lib/x86/vm.o > cflatobjs += lib/x86/fwcfg.o > cflatobjs += lib/x86/apic.o > @@ -20,15 +16,17 @@ $(libcflat): LDFLAGS += -nostdlib > $(libcflat): CFLAGS += -ffreestanding -I lib > > CFLAGS += -m$(bits) > +CFLAGS += -O1 > > libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name) > > FLATLIBS = lib/libcflat.a $(libgcc) > %.elf: %.o $(FLATLIBS) flat.lds > - $(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,flat.lds $(filter %.o, $^) $(FLATLIBS) > + $(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,flat.lds \ > + $(filter %.o, $^) $(FLATLIBS) > > %.flat: %.elf > - objcopy -O elf32-i386 $^ $@ > + $(OBJCOPY) -O elf32-i386 $^ $@ > > tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \ > $(TEST_DIR)/smptest.flat $(TEST_DIR)/port80.flat \ > @@ -105,7 +103,7 @@ $(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o > > arch_clean: > $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \ > - $(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o > + $(TEST_DIR)/.*.d lib/x86/.*.d > > api/%.o: CFLAGS += -m32 > > diff --git a/run_tests.sh b/run_tests.sh > index 55ecac5bed3a4..f373c533b75b2 100755 > --- a/run_tests.sh > +++ b/run_tests.sh > @@ -27,7 +27,7 @@ function run() > return > fi > > - cmdline="./x86-run $kernel -smp $smp -display none $opts" > + cmdline="./x86-run $kernel -smp $smp $opts" > if [ $verbose != 0 ]; then > echo $cmdline > fi > -- > 1.8.1.4 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm -- Christoffer