[PATCH] kvmtool: Makefile: allow overriding CC and LD

[PATCH] kvmtool: Makefile: allow overriding CC and LD

[Andre Przywara]

Currently we set CC unconditionally to ${CROSS_COMPILE}gcc, the same
for LD.
Allow people to override the compiler name by specifying it explicitly
on the command line or via the environment.
Beside calling a certain compiler binary this allows to pass in
options to the compiler, which lets us get rid of the PowerPC
overrides in the Makefile. Possible uses:
$ make CC="gcc -m64" LD="ld -melf64ppc"
(build kvmtool on a PowerPC toolchain defaulting to 32-bit)
$ make CC="gcc -m32" LD="ld -melf_i386"
(build a 32-bit binary on a multilib-enabled x86-64 compiler)

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 6110b8e..888bee5 100644
--- a/Makefile
+++ b/Makefile
@@ -14,9 +14,13 @@ export E Q
 include config/utilities.mak
 include config/feature-tests.mak
 
-CC	:= $(CROSS_COMPILE)gcc
+ifeq ($(origin CC), default)
+	CC	:= $(CROSS_COMPILE)gcc
+endif
 CFLAGS	:=
-LD	:= $(CROSS_COMPILE)ld
+ifeq ($(origin LD), default)
+	LD	:= $(CROSS_COMPILE)ld
+endif
 LDFLAGS	:=
 
 FIND	:= find
@@ -148,8 +152,6 @@ ifeq ($(ARCH), powerpc)
 	OBJS	+= powerpc/spapr_pci.o
 	OBJS	+= powerpc/xics.o
 	ARCH_INCLUDE := powerpc/include
-	CFLAGS 	+= -m64
-	LDFLAGS += -m elf64ppc
 
 	ARCH_WANT_LIBFDT := y
 endif
-- 
2.3.5

Re: [PATCH] kvmtool: Makefile: allow overriding CC and LD

[Michael Ellerman]

On Thu, 2015-06-18 at 16:50 +0100, Andre Przywara wrote:
> Currently we set CC unconditionally to ${CROSS_COMPILE}gcc, the same
> for LD.
> Allow people to override the compiler name by specifying it explicitly
> on the command line or via the environment.
> Beside calling a certain compiler binary this allows to pass in
> options to the compiler, which lets us get rid of the PowerPC
> overrides in the Makefile. Possible uses:
> $ make CC="gcc -m64" LD="ld -melf64ppc"
> (build kvmtool on a PowerPC toolchain defaulting to 32-bit)
> $ make CC="gcc -m32" LD="ld -melf_i386"
> (build a 32-bit binary on a multilib-enabled x86-64 compiler)


I'm not a big fan of that.

Your examples are all about overriding CFLAGS and LDFLAGS, not CC and LD. So
if anything you should be allowing that. Adding flags to CC and LD is asking
for trouble.

cheers

Re: [PATCH] kvmtool: Makefile: allow overriding CC and LD

[Paolo Bonzini]

On 18/06/2015 17:50, Andre Przywara wrote:
> Currently we set CC unconditionally to ${CROSS_COMPILE}gcc, the same
> for LD.
> Allow people to override the compiler name by specifying it explicitly
> on the command line or via the environment.
> Beside calling a certain compiler binary this allows to pass in
> options to the compiler, which lets us get rid of the PowerPC
> overrides in the Makefile. Possible uses:
> $ make CC="gcc -m64" LD="ld -melf64ppc"
> (build kvmtool on a PowerPC toolchain defaulting to 32-bit)
> $ make CC="gcc -m32" LD="ld -melf_i386"
> (build a 32-bit binary on a multilib-enabled x86-64 compiler)
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6110b8e..888bee5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -14,9 +14,13 @@ export E Q
>  include config/utilities.mak
>  include config/feature-tests.mak
>  
> -CC	:= $(CROSS_COMPILE)gcc
> +ifeq ($(origin CC), default)
> +	CC	:= $(CROSS_COMPILE)gcc
> +endif
>  CFLAGS	:=
> -LD	:= $(CROSS_COMPILE)ld
> +ifeq ($(origin LD), default)
> +	LD	:= $(CROSS_COMPILE)ld
> +endif

I know zero about the kvmtool build system, but that ought not to be
necessary.  The make manual says:

"If a variable has been set with a command argument (*note Overriding
Variables: Overriding.), then ordinary assignments in the makefile are
ignored."

Paolo

>  LDFLAGS	:=
>  
>  FIND	:= find
> @@ -148,8 +152,6 @@ ifeq ($(ARCH), powerpc)
>  	OBJS	+= powerpc/spapr_pci.o
>  	OBJS	+= powerpc/xics.o
>  	ARCH_INCLUDE := powerpc/include
> -	CFLAGS 	+= -m64
> -	LDFLAGS += -m elf64ppc
>  
>  	ARCH_WANT_LIBFDT := y
>  endif
> 

Re: [PATCH] kvmtool: Makefile: allow overriding CC and LD

[Andre Przywara]

Hi Michael,

On 19/06/15 02:14, Michael Ellerman wrote:
> On Thu, 2015-06-18 at 16:50 +0100, Andre Przywara wrote:
>> Currently we set CC unconditionally to ${CROSS_COMPILE}gcc, the same
>> for LD.
>> Allow people to override the compiler name by specifying it explicitly
>> on the command line or via the environment.
>> Beside calling a certain compiler binary this allows to pass in
>> options to the compiler, which lets us get rid of the PowerPC
>> overrides in the Makefile. Possible uses:
>> $ make CC="gcc -m64" LD="ld -melf64ppc"
>> (build kvmtool on a PowerPC toolchain defaulting to 32-bit)
>> $ make CC="gcc -m32" LD="ld -melf_i386"
>> (build a 32-bit binary on a multilib-enabled x86-64 compiler)
> 
> 
> I'm not a big fan of that.
> 
> Your examples are all about overriding CFLAGS and LDFLAGS, not CC and LD. So
> if anything you should be allowing that. Adding flags to CC and LD is asking
> for trouble.

Will just disabled overriding CFLAGS and LDFLAGS, I think because
kvmtool inherited some C nastiness from the kernel, which does not
compile with random flags set (CFLAGS=-std=gnu99 was the one the broke it).
Maybe we should revisit that, either fix the code to be more robust to
comply with various standards or document that you should not have
CFLAGS set. Then allow overriding CFLAGS again.

But I thought that overriding CC is common practise - if you want to
select a different compiler, that is. Using a different bitness seems a
lot like a different compiler to me, same with different endianness.
I think I saw quite some examples on the web about using CC="gcc -m32".

I agree that abusing CC to pass optimization options to the compiler is
not good, but for kvmtool's Makefile I don't see how adding flags to CC
would hurt.

Cheers,
Andre.

Re: [PATCH] kvmtool: Makefile: allow overriding CC and LD

[Andre Przywara]

Hi Paolo,

On 19/06/15 10:59, Paolo Bonzini wrote:
> 
> 
> On 18/06/2015 17:50, Andre Przywara wrote:
>> Currently we set CC unconditionally to ${CROSS_COMPILE}gcc, the same
>> for LD.
>> Allow people to override the compiler name by specifying it explicitly
>> on the command line or via the environment.
>> Beside calling a certain compiler binary this allows to pass in
>> options to the compiler, which lets us get rid of the PowerPC
>> overrides in the Makefile. Possible uses:
>> $ make CC="gcc -m64" LD="ld -melf64ppc"
>> (build kvmtool on a PowerPC toolchain defaulting to 32-bit)
>> $ make CC="gcc -m32" LD="ld -melf_i386"
>> (build a 32-bit binary on a multilib-enabled x86-64 compiler)
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  Makefile | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 6110b8e..888bee5 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -14,9 +14,13 @@ export E Q
>>  include config/utilities.mak
>>  include config/feature-tests.mak
>>  
>> -CC	:= $(CROSS_COMPILE)gcc
>> +ifeq ($(origin CC), default)
>> +	CC	:= $(CROSS_COMPILE)gcc
>> +endif
>>  CFLAGS	:=
>> -LD	:= $(CROSS_COMPILE)ld
>> +ifeq ($(origin LD), default)
>> +	LD	:= $(CROSS_COMPILE)ld
>> +endif
> 
> I know zero about the kvmtool build system, 

Well, it inherits a lot from the kernel ;-)

> but that ought not to be
> necessary.  The make manual says:
> 
> "If a variable has been set with a command argument (*note Overriding
> Variables: Overriding.), then ordinary assignments in the makefile are
> ignored."

Right, there was this nasty difference between "CC=gcc make" and
"make CC=gcc". So I agree that the latter works even without that patch.

Guess I was only looking at the environment here.
Paolo, thanks for pointing out!

Will, please ignore this patch then.

Cheers,
Andre.

> 
> Paolo
> 
>>  LDFLAGS	:=
>>  
>>  FIND	:= find
>> @@ -148,8 +152,6 @@ ifeq ($(ARCH), powerpc)
>>  	OBJS	+= powerpc/spapr_pci.o
>>  	OBJS	+= powerpc/xics.o
>>  	ARCH_INCLUDE := powerpc/include
>> -	CFLAGS 	+= -m64
>> -	LDFLAGS += -m elf64ppc
>>  
>>  	ARCH_WANT_LIBFDT := y
>>  endif
>>
>