Re: [PATCH 2/9] makefile and run_tests tweaks

[Christoffer Dall <christoffer.dall@linaro.org>]

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 <drjones@redhat.com>
> ---
>  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