diff mbox

external: Fix cross compilation issue

Message ID CAOSf1CF_xobGu9rHLw2XSmCA=iwGAc=Or0oBJNWcvS91Q-ND+Q@mail.gmail.com
State Rejected
Headers show

Commit Message

Oliver O'Halloran July 28, 2016, 12:23 p.m. UTC 
On Thu, Jul 28, 2016 at 8:40 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
> On Thu, Jul 28, 2016 at 03:05:38PM +0530, Vasant Hegde wrote:
>> For some reason Makefile thinks compiler variables like CC is already
>> assigned and ignores CROSS_COMPILE flags. Hence I'm not able to generate
>> arm binary on x86.
>>
>> Use default assignment operator instead of conditional assignment
>> operator (?=) in Makefile.
>>
>> Fixes: 3137d249 (pflash: Allow building under yocto.)
>> CC: Patrick Williams <patrick@stwcx.xyz>
>> CC: Stewart Smith <stewart@linux.vnet.ibm.com>
>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> ---
>>  external/common/rules.mk | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/external/common/rules.mk b/external/common/rules.mk
>> index bb12fd5..416a40d 100644
>> --- a/external/common/rules.mk
>> +++ b/external/common/rules.mk
>> @@ -1,5 +1,5 @@
>> -CC ?= $(CROSS_COMPILE)gcc
>> -LD ?= $(CROSS_COMPILE)ld
>> +CC = $(CROSS_COMPILE)gcc
>> +LD = $(CROSS_COMPILE)ld
>
> This effectively undoes the change I made in 3137d249, so I'm not sure
> why we would want it or why it is needed.

make automatically defines CC so the ?= assignment is never done. You
can get the desired
behaviour with and ifdef block though:



> Yocto sets CC and LD directly and actually adds some of the CFLAGS onto
> the CC variable (for right or wrong).  Thus it doesn't use CROSS_COMPILE
> directly.  Also, by forcing 'CC = $(CROSS_COMPILE)gcc' we preclude the
> use of clang.

Forcing the assignment is a bad idea, but keep in mind that most of us
aren't using
Yocto. I hit this problem while trying to build a copy of pflash for
an unopened BMC
and needed to use their toolchain. Manually setting CC/LD from the command line
works, but the external/ tools should respect CROSS_COMPILE for consistency with
skiboot itself and linux.

>
>>  ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")
>>
>>  ifeq ($(ARCH),ARCH_ARM)
>> --
>> 2.5.5
>>
>
> --
> Patrick Williams
>
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
>

Comments

Patrick Williams July 28, 2016, 5:05 p.m. UTC | #1 
On Thu, Jul 28, 2016 at 10:23:05PM +1000, oliver wrote:
> On Thu, Jul 28, 2016 at 8:40 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
> > On Thu, Jul 28, 2016 at 03:05:38PM +0530, Vasant Hegde wrote:
> >> For some reason Makefile thinks compiler variables like CC is already
> >> assigned and ignores CROSS_COMPILE flags. Hence I'm not able to generate
> >> arm binary on x86.
> >>
> >> Use default assignment operator instead of conditional assignment
> >> operator (?=) in Makefile.
> >>
> >> Fixes: 3137d249 (pflash: Allow building under yocto.)
> >> CC: Patrick Williams <patrick@stwcx.xyz>
> >> CC: Stewart Smith <stewart@linux.vnet.ibm.com>
> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> >> ---
> >>  external/common/rules.mk | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/external/common/rules.mk b/external/common/rules.mk
> >> index bb12fd5..416a40d 100644
> >> --- a/external/common/rules.mk
> >> +++ b/external/common/rules.mk
> >> @@ -1,5 +1,5 @@
> >> -CC ?= $(CROSS_COMPILE)gcc
> >> -LD ?= $(CROSS_COMPILE)ld
> >> +CC = $(CROSS_COMPILE)gcc
> >> +LD = $(CROSS_COMPILE)ld
> >
> > This effectively undoes the change I made in 3137d249, so I'm not sure
> > why we would want it or why it is needed.
> 
> make automatically defines CC so the ?= assignment is never done. You
> can get the desired
> behaviour with and ifdef block though:
> 
> diff --git a/external/common/rules.mk b/external/common/rules.mk
> index 5558cd3..4dd7942 100644
> --- a/external/common/rules.mk
> +++ b/external/common/rules.mk
> @@ -1,5 +1,8 @@
> -CC ?= $(CROSS_COMPILE)gcc
> -LD ?= $(CROSS_COMPILE)ld
> +ifdef CROSS_COMPILE
> +CC = $(CROSS_COMPILE)gcc
> +LD = $(CROSS_COMPILE)ld
> +endif
> +

I don't think that is sufficient for Yocto either.  The current
environment variables for a qemux86-64 target are:

CROSS_COMPILE=x86_64-openbmc-linux-
CC=x86_64-openbmc-linux-gcc -m64 -march=core2 -mtune=core2 -msse3
       -mfpmath=sse
       --sysroot=...sdk/sysroots/core2-64-openbmc-linux
LD=x86_64-openbmc-linux-ld
        --sysroot=...sdk/sysroots/core2-64-openbmc-linux

An ARM target would have something similar.

As you see both CROSS_COMPILE and CC/LD are set, but CC/LD are not
as simple as $(CROSS_COMPILE)gcc.  Also, by forcing gcc whenever
CROSS_COMPILE is set, you end up precluding the use of clang, which is
supported by Yocto.

>  ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")
> 
>  ifeq ($(ARCH),ARCH_ARM)
> 
> 
> > Yocto sets CC and LD directly and actually adds some of the CFLAGS onto
> > the CC variable (for right or wrong).  Thus it doesn't use CROSS_COMPILE
> > directly.  Also, by forcing 'CC = $(CROSS_COMPILE)gcc' we preclude the
> > use of clang.
> 
> Forcing the assignment is a bad idea, but keep in mind that most of us
> aren't using
> Yocto. I hit this problem while trying to build a copy of pflash for
> an unopened BMC
> and needed to use their toolchain. Manually setting CC/LD from the command line
> works, but the external/ tools should respect CROSS_COMPILE for consistency with
> skiboot itself and linux.
> 
> >
> >>  ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")
> >>
> >>  ifeq ($(ARCH),ARCH_ARM)
> >> --
> >> 2.5.5
> >>
> >
> > --
> > Patrick Williams
> >
> > _______________________________________________
> > Skiboot mailing list
> > Skiboot@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/skiboot
> >
Oliver O'Halloran July 29, 2016, 1:31 a.m. UTC | #2 
On Fri, Jul 29, 2016 at 3:05 AM, Patrick Williams <patrick@stwcx.xyz> wrote:
> On Thu, Jul 28, 2016 at 10:23:05PM +1000, oliver wrote:
>> On Thu, Jul 28, 2016 at 8:40 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
>> > On Thu, Jul 28, 2016 at 03:05:38PM +0530, Vasant Hegde wrote:
>> >> For some reason Makefile thinks compiler variables like CC is already
>> >> assigned and ignores CROSS_COMPILE flags. Hence I'm not able to generate
>> >> arm binary on x86.
>> >>
>> >> Use default assignment operator instead of conditional assignment
>> >> operator (?=) in Makefile.
>> >>
>> >> Fixes: 3137d249 (pflash: Allow building under yocto.)
>> >> CC: Patrick Williams <patrick@stwcx.xyz>
>> >> CC: Stewart Smith <stewart@linux.vnet.ibm.com>
>> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> >> ---
>> >>  external/common/rules.mk | 4 ++--
>> >>  1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/external/common/rules.mk b/external/common/rules.mk
>> >> index bb12fd5..416a40d 100644
>> >> --- a/external/common/rules.mk
>> >> +++ b/external/common/rules.mk
>> >> @@ -1,5 +1,5 @@
>> >> -CC ?= $(CROSS_COMPILE)gcc
>> >> -LD ?= $(CROSS_COMPILE)ld
>> >> +CC = $(CROSS_COMPILE)gcc
>> >> +LD = $(CROSS_COMPILE)ld
>> >
>> > This effectively undoes the change I made in 3137d249, so I'm not sure
>> > why we would want it or why it is needed.
>>
>> make automatically defines CC so the ?= assignment is never done. You
>> can get the desired
>> behaviour with and ifdef block though:
>>
>> diff --git a/external/common/rules.mk b/external/common/rules.mk
>> index 5558cd3..4dd7942 100644
>> --- a/external/common/rules.mk
>> +++ b/external/common/rules.mk
>> @@ -1,5 +1,8 @@
>> -CC ?= $(CROSS_COMPILE)gcc
>> -LD ?= $(CROSS_COMPILE)ld
>> +ifdef CROSS_COMPILE
>> +CC = $(CROSS_COMPILE)gcc
>> +LD = $(CROSS_COMPILE)ld
>> +endif
>> +
>
> I don't think that is sufficient for Yocto either.  The current
> environment variables for a qemux86-64 target are:
>
> CROSS_COMPILE=x86_64-openbmc-linux-
> CC=x86_64-openbmc-linux-gcc -m64 -march=core2 -mtune=core2 -msse3
>        -mfpmath=sse
>        --sysroot=...sdk/sysroots/core2-64-openbmc-linux
> LD=x86_64-openbmc-linux-ld
>         --sysroot=...sdk/sysroots/core2-64-openbmc-linux
>
> An ARM target would have something similar.
>
> As you see both CROSS_COMPILE and CC/LD are set, but CC/LD are not
> as simple as $(CROSS_COMPILE)gcc.  Also, by forcing gcc whenever
> CROSS_COMPILE is set, you end up precluding the use of clang, which is
> supported by Yocto.

This is Yocto problem and it should be worked around inside of Yocto.
Setting CROSS_COMPILE is essentially telling the build system that
it's free to modify CC. If Yocto wants to manage CC itself, then it
shouldn't be setting CROSS_COMPILE because there is no sane way to
make that interaction work.

>
>>  ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")
>>
>>  ifeq ($(ARCH),ARCH_ARM)
>>
>>
>> > Yocto sets CC and LD directly and actually adds some of the CFLAGS onto
>> > the CC variable (for right or wrong).  Thus it doesn't use CROSS_COMPILE
>> > directly.  Also, by forcing 'CC = $(CROSS_COMPILE)gcc' we preclude the
>> > use of clang.
>>
>> Forcing the assignment is a bad idea, but keep in mind that most of us
>> aren't using
>> Yocto. I hit this problem while trying to build a copy of pflash for
>> an unopened BMC
>> and needed to use their toolchain. Manually setting CC/LD from the command line
>> works, but the external/ tools should respect CROSS_COMPILE for consistency with
>> skiboot itself and linux.
>>
>> >
>> >>  ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")
>> >>
>> >>  ifeq ($(ARCH),ARCH_ARM)
>> >> --
>> >> 2.5.5
>> >>
>> >
>> > --
>> > Patrick Williams
>> >
>> > _______________________________________________
>> > Skiboot mailing list
>> > Skiboot@lists.ozlabs.org
>> > https://lists.ozlabs.org/listinfo/skiboot
>> >
>
> --
> Patrick Williams
Vasant Hegde Aug. 3, 2016, 5:43 a.m. UTC | #3 
On 07/28/2016 05:53 PM, oliver wrote:
> On Thu, Jul 28, 2016 at 8:40 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
>> On Thu, Jul 28, 2016 at 03:05:38PM +0530, Vasant Hegde wrote:
>>> For some reason Makefile thinks compiler variables like CC is already
>>> assigned and ignores CROSS_COMPILE flags. Hence I'm not able to generate
>>> arm binary on x86.
>>>
>>> Use default assignment operator instead of conditional assignment
>>> operator (?=) in Makefile.
>>>
>>> Fixes: 3137d249 (pflash: Allow building under yocto.)
>>> CC: Patrick Williams <patrick@stwcx.xyz>
>>> CC: Stewart Smith <stewart@linux.vnet.ibm.com>
>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>>> ---
>>>   external/common/rules.mk | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/external/common/rules.mk b/external/common/rules.mk
>>> index bb12fd5..416a40d 100644
>>> --- a/external/common/rules.mk
>>> +++ b/external/common/rules.mk
>>> @@ -1,5 +1,5 @@
>>> -CC ?= $(CROSS_COMPILE)gcc
>>> -LD ?= $(CROSS_COMPILE)ld
>>> +CC = $(CROSS_COMPILE)gcc
>>> +LD = $(CROSS_COMPILE)ld
>>
>> This effectively undoes the change I made in 3137d249, so I'm not sure
>> why we would want it or why it is needed.
>
> make automatically defines CC so the ?= assignment is never done. You
> can get the desired
> behaviour with and ifdef block though:
>
> diff --git a/external/common/rules.mk b/external/common/rules.mk
> index 5558cd3..4dd7942 100644
> --- a/external/common/rules.mk
> +++ b/external/common/rules.mk
> @@ -1,5 +1,8 @@
> -CC ?= $(CROSS_COMPILE)gcc
> -LD ?= $(CROSS_COMPILE)ld
> +ifdef CROSS_COMPILE
> +CC = $(CROSS_COMPILE)gcc
> +LD = $(CROSS_COMPILE)ld
> +endif
> +

Looks better. I will send out v2.

-Vasant
diff mbox

Patch

diff --git a/external/common/rules.mk b/external/common/rules.mk
index 5558cd3..4dd7942 100644
--- a/external/common/rules.mk
+++ b/external/common/rules.mk
@@ -1,5 +1,8 @@ 
-CC ?= $(CROSS_COMPILE)gcc
-LD ?= $(CROSS_COMPILE)ld
+ifdef CROSS_COMPILE
+CC = $(CROSS_COMPILE)gcc
+LD = $(CROSS_COMPILE)ld
+endif
+
 ARCH := $(shell $(GET_ARCH) "$(CROSS_COMPILE)")

 ifeq ($(ARCH),ARCH_ARM)