kvmtool: Makefile: allow overriding CC and LD

Message ID 1434642647-3127-1-git-send-email-andre.przywara@arm.com
State Not Applicable, archived
Headers show

Commit Message

Andre Przywara June 18, 2015, 3:50 p.m.
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(-)

Comments

Michael Ellerman June 19, 2015, 1:14 a.m. | #1
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


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini June 19, 2015, 9:59 a.m. | #2
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
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andre Przywara June 19, 2015, 10 a.m. | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andre Przywara June 19, 2015, 10:23 a.m. | #4
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
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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