[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Fabrice Fontaine]

When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
will be used with HOSTCFLAGS which seems wrong

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 tools/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/Makefile b/tools/Makefile
index 12a3027e23..eadeba417d 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -272,6 +272,7 @@ subdir- += env
 
 ifneq ($(CROSS_BUILD_TOOLS),)
 override HOSTCC = $(CC)
+override HOSTCFLAGS = $(CFLAGS)
 
 quiet_cmd_crosstools_strip = STRIP   $^
       cmd_crosstools_strip = $(STRIP) $^; touch $@
-- 
2.20.1

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Tom Rini]

On Wed, May 01, 2019 at 03:08:25PM +0200, Fabrice Fontaine wrote:

> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> will be used with HOSTCFLAGS which seems wrong
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190503/de6aa4a2/attachment.sig>

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 01.05.19 15:08, Fabrice Fontaine wrote:
> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> will be used with HOSTCFLAGS which seems wrong
>
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>   tools/Makefile | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 12a3027e23..eadeba417d 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -272,6 +272,7 @@ subdir- += env
>
>   ifneq ($(CROSS_BUILD_TOOLS),)
>   override HOSTCC = $(CC)
> +override HOSTCFLAGS = $(CFLAGS)
>
>   quiet_cmd_crosstools_strip = STRIP   $^
>         cmd_crosstools_strip = $(STRIP) $^; touch $@
>

This eats - among other things - -O2, normally set in /Makefile. And that breaks
CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
(!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
simply be reverted (which is what I'm doing locally in order to fix my builds).

Jan

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Fabrice Fontaine]

Hello,
Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
>
> On 01.05.19 15:08, Fabrice Fontaine wrote:
> > When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> > will be used with HOSTCFLAGS which seems wrong
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >   tools/Makefile | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 12a3027e23..eadeba417d 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -272,6 +272,7 @@ subdir- += env
> >
> >   ifneq ($(CROSS_BUILD_TOOLS),)
> >   override HOSTCC = $(CC)
> > +override HOSTCFLAGS = $(CFLAGS)
> >
> >   quiet_cmd_crosstools_strip = STRIP   $^
> >         cmd_crosstools_strip = $(STRIP) $^; touch $@
> >
>
> This eats - among other things - -O2, normally set in /Makefile. And that breaks
> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
> simply be reverted (which is what I'm doing locally in order to fix my builds).
I don't think this patch should be reverted, I sent it to fix a
cross-compilation build issue with host-openssl.

Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
sets HOSTCC=$(CC) but it forgets
to override HOSTCFLAGS, though, so the tools are built with target
compiler but host CFLAGS.

This raises a build failure if host-openssl is built before uboot-tools because
uboot-tools links with openssl headers from host which depends on pthread.h

More information can be found on buildroot's patchwork here:
- https://patchwork.ozlabs.org/patch/1092227
- https://patchwork.ozlabs.org/patch/1093385
>
> Jan

Best Regards,

Fabrice

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 25.08.19 15:43, Fabrice Fontaine wrote:
> Hello,
> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>
>> On 01.05.19 15:08, Fabrice Fontaine wrote:
>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
>>> will be used with HOSTCFLAGS which seems wrong
>>>
>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>>> ---
>>>    tools/Makefile | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/tools/Makefile b/tools/Makefile
>>> index 12a3027e23..eadeba417d 100644
>>> --- a/tools/Makefile
>>> +++ b/tools/Makefile
>>> @@ -272,6 +272,7 @@ subdir- += env
>>>
>>>    ifneq ($(CROSS_BUILD_TOOLS),)
>>>    override HOSTCC = $(CC)
>>> +override HOSTCFLAGS = $(CFLAGS)
>>>
>>>    quiet_cmd_crosstools_strip = STRIP   $^
>>>          cmd_crosstools_strip = $(STRIP) $^; touch $@
>>>
>>
>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
>> simply be reverted (which is what I'm doing locally in order to fix my builds).
> I don't think this patch should be reverted, I sent it to fix a
> cross-compilation build issue with host-openssl.
>
> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
> sets HOSTCC=$(CC) but it forgets
> to override HOSTCFLAGS, though, so the tools are built with target
> compiler but host CFLAGS.
>
> This raises a build failure if host-openssl is built before uboot-tools because
> uboot-tools links with openssl headers from host which depends on pthread.h
>
> More information can be found on buildroot's patchwork here:
> - https://patchwork.ozlabs.org/patch/1092227
> - https://patchwork.ozlabs.org/patch/1093385
>>
>> Jan
>
> Best Regards,
>
> Fabrice
>

So what is your suggestion to fix the O2-regression best?

Jan

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Fabrice Fontaine]

Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
>
> On 25.08.19 15:43, Fabrice Fontaine wrote:
> > Hello,
> > Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
> >>
> >> On 01.05.19 15:08, Fabrice Fontaine wrote:
> >>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> >>> will be used with HOSTCFLAGS which seems wrong
> >>>
> >>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> >>> ---
> >>>    tools/Makefile | 1 +
> >>>    1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/tools/Makefile b/tools/Makefile
> >>> index 12a3027e23..eadeba417d 100644
> >>> --- a/tools/Makefile
> >>> +++ b/tools/Makefile
> >>> @@ -272,6 +272,7 @@ subdir- += env
> >>>
> >>>    ifneq ($(CROSS_BUILD_TOOLS),)
> >>>    override HOSTCC = $(CC)
> >>> +override HOSTCFLAGS = $(CFLAGS)
> >>>
> >>>    quiet_cmd_crosstools_strip = STRIP   $^
> >>>          cmd_crosstools_strip = $(STRIP) $^; touch $@
> >>>
> >>
> >> This eats - among other things - -O2, normally set in /Makefile. And that breaks
> >> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
> >> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
> >> simply be reverted (which is what I'm doing locally in order to fix my builds).
> > I don't think this patch should be reverted, I sent it to fix a
> > cross-compilation build issue with host-openssl.
> >
> > Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
> > sets HOSTCC=$(CC) but it forgets
> > to override HOSTCFLAGS, though, so the tools are built with target
> > compiler but host CFLAGS.
> >
> > This raises a build failure if host-openssl is built before uboot-tools because
> > uboot-tools links with openssl headers from host which depends on pthread.h
> >
> > More information can be found on buildroot's patchwork here:
> > - https://patchwork.ozlabs.org/patch/1092227
> > - https://patchwork.ozlabs.org/patch/1093385
> >>
> >> Jan
> >
> > Best Regards,
> >
> > Fabrice
> >
>
> So what is your suggestion to fix the O2-regression best?
I'm far from being an expert in uboot so please excuse me if my answer
contains some mistakes.
However, IMHO, we should not pass -O2 or any other optimization flags.
From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
for the target through cross-tools) will (or should?) contain -O2 or
-Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
However, I do understand that this is a change in the former behavior
that was always passing -O2 in HOSTCFLAGS.
So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
tools/Makefile (after the override).
However, I have to confess that I don't understand why the build
breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
>
> Jan

Fabrice

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 25.08.19 17:13, Fabrice Fontaine wrote:
> Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>
>> On 25.08.19 15:43, Fabrice Fontaine wrote:
>>> Hello,
>>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>
>>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
>>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
>>>>> will be used with HOSTCFLAGS which seems wrong
>>>>>
>>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>>>>> ---
>>>>>     tools/Makefile | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>> index 12a3027e23..eadeba417d 100644
>>>>> --- a/tools/Makefile
>>>>> +++ b/tools/Makefile
>>>>> @@ -272,6 +272,7 @@ subdir- += env
>>>>>
>>>>>     ifneq ($(CROSS_BUILD_TOOLS),)
>>>>>     override HOSTCC = $(CC)
>>>>> +override HOSTCFLAGS = $(CFLAGS)
>>>>>
>>>>>     quiet_cmd_crosstools_strip = STRIP   $^
>>>>>           cmd_crosstools_strip = $(STRIP) $^; touch $@
>>>>>
>>>>
>>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
>>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
>>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
>>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
>>> I don't think this patch should be reverted, I sent it to fix a
>>> cross-compilation build issue with host-openssl.
>>>
>>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
>>> sets HOSTCC=$(CC) but it forgets
>>> to override HOSTCFLAGS, though, so the tools are built with target
>>> compiler but host CFLAGS.
>>>
>>> This raises a build failure if host-openssl is built before uboot-tools because
>>> uboot-tools links with openssl headers from host which depends on pthread.h
>>>
>>> More information can be found on buildroot's patchwork here:
>>> - https://patchwork.ozlabs.org/patch/1092227
>>> - https://patchwork.ozlabs.org/patch/1093385
>>>>
>>>> Jan
>>>
>>> Best Regards,
>>>
>>> Fabrice
>>>
>>
>> So what is your suggestion to fix the O2-regression best?
> I'm far from being an expert in uboot so please excuse me if my answer
> contains some mistakes.
> However, IMHO, we should not pass -O2 or any other optimization flags.
>  From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
> for the target through cross-tools) will (or should?) contain -O2 or
> -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
> However, I do understand that this is a change in the former behavior
> that was always passing -O2 in HOSTCFLAGS.
> So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
> tools/Makefile (after the override).

This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
setting on its own - which it did prior to your change. I think top-level
HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
appending (and, thus, overwriting) settings but you cannot remove mandatory ones.

> However, I have to confess that I don't understand why the build
> breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."

IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
all code from tools/image-host.c which is not reachable then. When it's not
removed because of default optimization settings, we get linker errors because
common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
was preferred over #ifdef'ery, but it's broken now.

Jan

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Fabrice Fontaine]

Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
>
> On 25.08.19 17:13, Fabrice Fontaine wrote:
> > Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
> >>
> >> On 25.08.19 15:43, Fabrice Fontaine wrote:
> >>> Hello,
> >>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
> >>>>
> >>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
> >>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> >>>>> will be used with HOSTCFLAGS which seems wrong
> >>>>>
> >>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> >>>>> ---
> >>>>>     tools/Makefile | 1 +
> >>>>>     1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/tools/Makefile b/tools/Makefile
> >>>>> index 12a3027e23..eadeba417d 100644
> >>>>> --- a/tools/Makefile
> >>>>> +++ b/tools/Makefile
> >>>>> @@ -272,6 +272,7 @@ subdir- += env
> >>>>>
> >>>>>     ifneq ($(CROSS_BUILD_TOOLS),)
> >>>>>     override HOSTCC = $(CC)
> >>>>> +override HOSTCFLAGS = $(CFLAGS)
> >>>>>
> >>>>>     quiet_cmd_crosstools_strip = STRIP   $^
> >>>>>           cmd_crosstools_strip = $(STRIP) $^; touch $@
> >>>>>
> >>>>
> >>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
> >>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
> >>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
> >>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
> >>> I don't think this patch should be reverted, I sent it to fix a
> >>> cross-compilation build issue with host-openssl.
> >>>
> >>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
> >>> sets HOSTCC=$(CC) but it forgets
> >>> to override HOSTCFLAGS, though, so the tools are built with target
> >>> compiler but host CFLAGS.
> >>>
> >>> This raises a build failure if host-openssl is built before uboot-tools because
> >>> uboot-tools links with openssl headers from host which depends on pthread.h
> >>>
> >>> More information can be found on buildroot's patchwork here:
> >>> - https://patchwork.ozlabs.org/patch/1092227
> >>> - https://patchwork.ozlabs.org/patch/1093385
> >>>>
> >>>> Jan
> >>>
> >>> Best Regards,
> >>>
> >>> Fabrice
> >>>
> >>
> >> So what is your suggestion to fix the O2-regression best?
> > I'm far from being an expert in uboot so please excuse me if my answer
> > contains some mistakes.
> > However, IMHO, we should not pass -O2 or any other optimization flags.
> >  From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
> > for the target through cross-tools) will (or should?) contain -O2 or
> > -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
> > However, I do understand that this is a change in the former behavior
> > that was always passing -O2 in HOSTCFLAGS.
> > So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
> > tools/Makefile (after the override).
>
> This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
> setting on its own - which it did prior to your change. I think top-level
> HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
> appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
>
> > However, I have to confess that I don't understand why the build
> > breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
>
> IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
> all code from tools/image-host.c which is not reachable then. When it's not
> removed because of default optimization settings, we get linker errors because
> common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
> was preferred over #ifdef'ery, but it's broken now.
Thanks a lot for your explanation.
I was able to reproduce the issue and I think that
https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
should fixed it.
Can you review it? I can also directly send a PR.
>
> Jan
Fabrice

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 25.08.19 21:11, Fabrice Fontaine wrote:
> Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>
>> On 25.08.19 17:13, Fabrice Fontaine wrote:
>>> Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>
>>>> On 25.08.19 15:43, Fabrice Fontaine wrote:
>>>>> Hello,
>>>>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>>>
>>>>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
>>>>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
>>>>>>> will be used with HOSTCFLAGS which seems wrong
>>>>>>>
>>>>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>>>>>>> ---
>>>>>>>      tools/Makefile | 1 +
>>>>>>>      1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>>>> index 12a3027e23..eadeba417d 100644
>>>>>>> --- a/tools/Makefile
>>>>>>> +++ b/tools/Makefile
>>>>>>> @@ -272,6 +272,7 @@ subdir- += env
>>>>>>>
>>>>>>>      ifneq ($(CROSS_BUILD_TOOLS),)
>>>>>>>      override HOSTCC = $(CC)
>>>>>>> +override HOSTCFLAGS = $(CFLAGS)
>>>>>>>
>>>>>>>      quiet_cmd_crosstools_strip = STRIP   $^
>>>>>>>            cmd_crosstools_strip = $(STRIP) $^; touch $@
>>>>>>>
>>>>>>
>>>>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
>>>>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
>>>>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
>>>>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
>>>>> I don't think this patch should be reverted, I sent it to fix a
>>>>> cross-compilation build issue with host-openssl.
>>>>>
>>>>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
>>>>> sets HOSTCC=$(CC) but it forgets
>>>>> to override HOSTCFLAGS, though, so the tools are built with target
>>>>> compiler but host CFLAGS.
>>>>>
>>>>> This raises a build failure if host-openssl is built before uboot-tools because
>>>>> uboot-tools links with openssl headers from host which depends on pthread.h
>>>>>
>>>>> More information can be found on buildroot's patchwork here:
>>>>> - https://patchwork.ozlabs.org/patch/1092227
>>>>> - https://patchwork.ozlabs.org/patch/1093385
>>>>>>
>>>>>> Jan
>>>>>
>>>>> Best Regards,
>>>>>
>>>>> Fabrice
>>>>>
>>>>
>>>> So what is your suggestion to fix the O2-regression best?
>>> I'm far from being an expert in uboot so please excuse me if my answer
>>> contains some mistakes.
>>> However, IMHO, we should not pass -O2 or any other optimization flags.
>>>   From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
>>> for the target through cross-tools) will (or should?) contain -O2 or
>>> -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
>>> However, I do understand that this is a change in the former behavior
>>> that was always passing -O2 in HOSTCFLAGS.
>>> So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
>>> tools/Makefile (after the override).
>>
>> This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
>> setting on its own - which it did prior to your change. I think top-level
>> HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
>> appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
>>
>>> However, I have to confess that I don't understand why the build
>>> breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
>>
>> IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
>> all code from tools/image-host.c which is not reachable then. When it's not
>> removed because of default optimization settings, we get linker errors because
>> common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
>> was preferred over #ifdef'ery, but it's broken now.
> Thanks a lot for your explanation.
> I was able to reproduce the issue and I think that
> https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
> should fixed it.
> Can you review it? I can also directly send a PR.

I still think that dropping the content of HOSTCFLAGS is the actual bug. There
is more than -O2 that you lose. If you want your CFLAGS to be respected, append
them.

Jan

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Fabrice Fontaine]

Le lun. 26 août 2019 à 07:57, Jan Kiszka <jan.kiszka@web.de> a écrit :
>
> On 25.08.19 21:11, Fabrice Fontaine wrote:
> > Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
> >>
> >> On 25.08.19 17:13, Fabrice Fontaine wrote:
> >>> Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
> >>>>
> >>>> On 25.08.19 15:43, Fabrice Fontaine wrote:
> >>>>> Hello,
> >>>>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
> >>>>>>
> >>>>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
> >>>>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> >>>>>>> will be used with HOSTCFLAGS which seems wrong
> >>>>>>>
> >>>>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> >>>>>>> ---
> >>>>>>>      tools/Makefile | 1 +
> >>>>>>>      1 file changed, 1 insertion(+)
> >>>>>>>
> >>>>>>> diff --git a/tools/Makefile b/tools/Makefile
> >>>>>>> index 12a3027e23..eadeba417d 100644
> >>>>>>> --- a/tools/Makefile
> >>>>>>> +++ b/tools/Makefile
> >>>>>>> @@ -272,6 +272,7 @@ subdir- += env
> >>>>>>>
> >>>>>>>      ifneq ($(CROSS_BUILD_TOOLS),)
> >>>>>>>      override HOSTCC = $(CC)
> >>>>>>> +override HOSTCFLAGS = $(CFLAGS)
> >>>>>>>
> >>>>>>>      quiet_cmd_crosstools_strip = STRIP   $^
> >>>>>>>            cmd_crosstools_strip = $(STRIP) $^; touch $@
> >>>>>>>
> >>>>>>
> >>>>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
> >>>>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
> >>>>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
> >>>>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
> >>>>> I don't think this patch should be reverted, I sent it to fix a
> >>>>> cross-compilation build issue with host-openssl.
> >>>>>
> >>>>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
> >>>>> sets HOSTCC=$(CC) but it forgets
> >>>>> to override HOSTCFLAGS, though, so the tools are built with target
> >>>>> compiler but host CFLAGS.
> >>>>>
> >>>>> This raises a build failure if host-openssl is built before uboot-tools because
> >>>>> uboot-tools links with openssl headers from host which depends on pthread.h
> >>>>>
> >>>>> More information can be found on buildroot's patchwork here:
> >>>>> - https://patchwork.ozlabs.org/patch/1092227
> >>>>> - https://patchwork.ozlabs.org/patch/1093385
> >>>>>>
> >>>>>> Jan
> >>>>>
> >>>>> Best Regards,
> >>>>>
> >>>>> Fabrice
> >>>>>
> >>>>
> >>>> So what is your suggestion to fix the O2-regression best?
> >>> I'm far from being an expert in uboot so please excuse me if my answer
> >>> contains some mistakes.
> >>> However, IMHO, we should not pass -O2 or any other optimization flags.
> >>>   From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
> >>> for the target through cross-tools) will (or should?) contain -O2 or
> >>> -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
> >>> However, I do understand that this is a change in the former behavior
> >>> that was always passing -O2 in HOSTCFLAGS.
> >>> So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
> >>> tools/Makefile (after the override).
> >>
> >> This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
> >> setting on its own - which it did prior to your change. I think top-level
> >> HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
> >> appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
> >>
> >>> However, I have to confess that I don't understand why the build
> >>> breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
> >>
> >> IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
> >> all code from tools/image-host.c which is not reachable then. When it's not
> >> removed because of default optimization settings, we get linker errors because
> >> common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
> >> was preferred over #ifdef'ery, but it's broken now.
> > Thanks a lot for your explanation.
> > I was able to reproduce the issue and I think that
> > https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
> > should fixed it.
> > Can you review it? I can also directly send a PR.
>
> I still think that dropping the content of HOSTCFLAGS is the actual bug. There
> is more than -O2 that you lose. If you want your CFLAGS to be respected, append
> them.
I add Arnout to the list to get his feedback as he is the one that
suggested this solution during review of
https://patchwork.ozlabs.org/patch/1092227.
>
> Jan
Fabrice

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 26.08.19 08:43, Fabrice Fontaine wrote:
> Le lun. 26 août 2019 à 07:57, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>
>> On 25.08.19 21:11, Fabrice Fontaine wrote:
>>> Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>
>>>> On 25.08.19 17:13, Fabrice Fontaine wrote:
>>>>> Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>>>
>>>>>> On 25.08.19 15:43, Fabrice Fontaine wrote:
>>>>>>> Hello,
>>>>>>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>>>>>
>>>>>>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
>>>>>>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
>>>>>>>>> will be used with HOSTCFLAGS which seems wrong
>>>>>>>>>
>>>>>>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>>>>>>>>> ---
>>>>>>>>>       tools/Makefile | 1 +
>>>>>>>>>       1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>>>>>> index 12a3027e23..eadeba417d 100644
>>>>>>>>> --- a/tools/Makefile
>>>>>>>>> +++ b/tools/Makefile
>>>>>>>>> @@ -272,6 +272,7 @@ subdir- += env
>>>>>>>>>
>>>>>>>>>       ifneq ($(CROSS_BUILD_TOOLS),)
>>>>>>>>>       override HOSTCC = $(CC)
>>>>>>>>> +override HOSTCFLAGS = $(CFLAGS)
>>>>>>>>>
>>>>>>>>>       quiet_cmd_crosstools_strip = STRIP   $^
>>>>>>>>>             cmd_crosstools_strip = $(STRIP) $^; touch $@
>>>>>>>>>
>>>>>>>>
>>>>>>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
>>>>>>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
>>>>>>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
>>>>>>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
>>>>>>> I don't think this patch should be reverted, I sent it to fix a
>>>>>>> cross-compilation build issue with host-openssl.
>>>>>>>
>>>>>>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
>>>>>>> sets HOSTCC=$(CC) but it forgets
>>>>>>> to override HOSTCFLAGS, though, so the tools are built with target
>>>>>>> compiler but host CFLAGS.
>>>>>>>
>>>>>>> This raises a build failure if host-openssl is built before uboot-tools because
>>>>>>> uboot-tools links with openssl headers from host which depends on pthread.h
>>>>>>>
>>>>>>> More information can be found on buildroot's patchwork here:
>>>>>>> - https://patchwork.ozlabs.org/patch/1092227
>>>>>>> - https://patchwork.ozlabs.org/patch/1093385
>>>>>>>>
>>>>>>>> Jan
>>>>>>>
>>>>>>> Best Regards,
>>>>>>>
>>>>>>> Fabrice
>>>>>>>
>>>>>>
>>>>>> So what is your suggestion to fix the O2-regression best?
>>>>> I'm far from being an expert in uboot so please excuse me if my answer
>>>>> contains some mistakes.
>>>>> However, IMHO, we should not pass -O2 or any other optimization flags.
>>>>>    From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
>>>>> for the target through cross-tools) will (or should?) contain -O2 or
>>>>> -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
>>>>> However, I do understand that this is a change in the former behavior
>>>>> that was always passing -O2 in HOSTCFLAGS.
>>>>> So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
>>>>> tools/Makefile (after the override).
>>>>
>>>> This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
>>>> setting on its own - which it did prior to your change. I think top-level
>>>> HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
>>>> appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
>>>>
>>>>> However, I have to confess that I don't understand why the build
>>>>> breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
>>>>
>>>> IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
>>>> all code from tools/image-host.c which is not reachable then. When it's not
>>>> removed because of default optimization settings, we get linker errors because
>>>> common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
>>>> was preferred over #ifdef'ery, but it's broken now.
>>> Thanks a lot for your explanation.
>>> I was able to reproduce the issue and I think that
>>> https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
>>> should fixed it.
>>> Can you review it? I can also directly send a PR.
>>
>> I still think that dropping the content of HOSTCFLAGS is the actual bug. There
>> is more than -O2 that you lose. If you want your CFLAGS to be respected, append
>> them.
> I add Arnout to the list to get his feedback as he is the one that
> suggested this solution during review of
> https://patchwork.ozlabs.org/patch/1092227.

Nothing happened, bug is still present in master. Any suggestions how to
resolve it?

Jan

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Tom Rini]

On Thu, Nov 14, 2019 at 05:28:06PM +0100, Jan Kiszka wrote:
> On 26.08.19 08:43, Fabrice Fontaine wrote:
> > Le lun. 26 août 2019 à 07:57, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > 
> > > On 25.08.19 21:11, Fabrice Fontaine wrote:
> > > > Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > 
> > > > > On 25.08.19 17:13, Fabrice Fontaine wrote:
> > > > > > Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > > > 
> > > > > > > On 25.08.19 15:43, Fabrice Fontaine wrote:
> > > > > > > > Hello,
> > > > > > > > Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > > > > > 
> > > > > > > > > On 01.05.19 15:08, Fabrice Fontaine wrote:
> > > > > > > > > > When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> > > > > > > > > > will be used with HOSTCFLAGS which seems wrong
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > >       tools/Makefile | 1 +
> > > > > > > > > >       1 file changed, 1 insertion(+)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/tools/Makefile b/tools/Makefile
> > > > > > > > > > index 12a3027e23..eadeba417d 100644
> > > > > > > > > > --- a/tools/Makefile
> > > > > > > > > > +++ b/tools/Makefile
> > > > > > > > > > @@ -272,6 +272,7 @@ subdir- += env
> > > > > > > > > > 
> > > > > > > > > >       ifneq ($(CROSS_BUILD_TOOLS),)
> > > > > > > > > >       override HOSTCC = $(CC)
> > > > > > > > > > +override HOSTCFLAGS = $(CFLAGS)
> > > > > > > > > > 
> > > > > > > > > >       quiet_cmd_crosstools_strip = STRIP   $^
> > > > > > > > > >             cmd_crosstools_strip = $(STRIP) $^; touch $@
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > This eats - among other things - -O2, normally set in /Makefile. And that breaks
> > > > > > > > > CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
> > > > > > > > > (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
> > > > > > > > > simply be reverted (which is what I'm doing locally in order to fix my builds).
> > > > > > > > I don't think this patch should be reverted, I sent it to fix a
> > > > > > > > cross-compilation build issue with host-openssl.
> > > > > > > > 
> > > > > > > > Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
> > > > > > > > sets HOSTCC=$(CC) but it forgets
> > > > > > > > to override HOSTCFLAGS, though, so the tools are built with target
> > > > > > > > compiler but host CFLAGS.
> > > > > > > > 
> > > > > > > > This raises a build failure if host-openssl is built before uboot-tools because
> > > > > > > > uboot-tools links with openssl headers from host which depends on pthread.h
> > > > > > > > 
> > > > > > > > More information can be found on buildroot's patchwork here:
> > > > > > > > - https://patchwork.ozlabs.org/patch/1092227
> > > > > > > > - https://patchwork.ozlabs.org/patch/1093385
> > > > > > > > > 
> > > > > > > > > Jan
> > > > > > > > 
> > > > > > > > Best Regards,
> > > > > > > > 
> > > > > > > > Fabrice
> > > > > > > > 
> > > > > > > 
> > > > > > > So what is your suggestion to fix the O2-regression best?
> > > > > > I'm far from being an expert in uboot so please excuse me if my answer
> > > > > > contains some mistakes.
> > > > > > However, IMHO, we should not pass -O2 or any other optimization flags.
> > > > > >    From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
> > > > > > for the target through cross-tools) will (or should?) contain -O2 or
> > > > > > -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
> > > > > > However, I do understand that this is a change in the former behavior
> > > > > > that was always passing -O2 in HOSTCFLAGS.
> > > > > > So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
> > > > > > tools/Makefile (after the override).
> > > > > 
> > > > > This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
> > > > > setting on its own - which it did prior to your change. I think top-level
> > > > > HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
> > > > > appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
> > > > > 
> > > > > > However, I have to confess that I don't understand why the build
> > > > > > breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
> > > > > 
> > > > > IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
> > > > > all code from tools/image-host.c which is not reachable then. When it's not
> > > > > removed because of default optimization settings, we get linker errors because
> > > > > common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
> > > > > was preferred over #ifdef'ery, but it's broken now.
> > > > Thanks a lot for your explanation.
> > > > I was able to reproduce the issue and I think that
> > > > https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
> > > > should fixed it.
> > > > Can you review it? I can also directly send a PR.
> > > 
> > > I still think that dropping the content of HOSTCFLAGS is the actual bug. There
> > > is more than -O2 that you lose. If you want your CFLAGS to be respected, append
> > > them.
> > I add Arnout to the list to get his feedback as he is the one that
> > suggested this solution during review of
> > https://patchwork.ozlabs.org/patch/1092227.
> 
> Nothing happened, bug is still present in master. Any suggestions how to
> resolve it?

OK, I've looked over the thread again.  Where and how are you hitting a
problem here again?  This area is indeed a bit funky and I see
OpenEmbedded has long been doing "shove everything we need in via CC, do
not use CFLAGS/etc".  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191114/9afd0360/attachment.sig>

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 14.11.19 17:41, Tom Rini wrote:
> On Thu, Nov 14, 2019 at 05:28:06PM +0100, Jan Kiszka wrote:
>> On 26.08.19 08:43, Fabrice Fontaine wrote:
>>> Le lun. 26 août 2019 à 07:57, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>
>>>> On 25.08.19 21:11, Fabrice Fontaine wrote:
>>>>> Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>>>
>>>>>> On 25.08.19 17:13, Fabrice Fontaine wrote:
>>>>>>> Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>>>>>
>>>>>>>> On 25.08.19 15:43, Fabrice Fontaine wrote:
>>>>>>>>> Hello,
>>>>>>>>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
>>>>>>>>>>
>>>>>>>>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
>>>>>>>>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
>>>>>>>>>>> will be used with HOSTCFLAGS which seems wrong
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>>>>>>>>>>> ---
>>>>>>>>>>>        tools/Makefile | 1 +
>>>>>>>>>>>        1 file changed, 1 insertion(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>>>>>>>> index 12a3027e23..eadeba417d 100644
>>>>>>>>>>> --- a/tools/Makefile
>>>>>>>>>>> +++ b/tools/Makefile
>>>>>>>>>>> @@ -272,6 +272,7 @@ subdir- += env
>>>>>>>>>>>
>>>>>>>>>>>        ifneq ($(CROSS_BUILD_TOOLS),)
>>>>>>>>>>>        override HOSTCC = $(CC)
>>>>>>>>>>> +override HOSTCFLAGS = $(CFLAGS)
>>>>>>>>>>>
>>>>>>>>>>>        quiet_cmd_crosstools_strip = STRIP   $^
>>>>>>>>>>>              cmd_crosstools_strip = $(STRIP) $^; touch $@
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
>>>>>>>>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
>>>>>>>>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
>>>>>>>>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
>>>>>>>>> I don't think this patch should be reverted, I sent it to fix a
>>>>>>>>> cross-compilation build issue with host-openssl.
>>>>>>>>>
>>>>>>>>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
>>>>>>>>> sets HOSTCC=$(CC) but it forgets
>>>>>>>>> to override HOSTCFLAGS, though, so the tools are built with target
>>>>>>>>> compiler but host CFLAGS.
>>>>>>>>>
>>>>>>>>> This raises a build failure if host-openssl is built before uboot-tools because
>>>>>>>>> uboot-tools links with openssl headers from host which depends on pthread.h
>>>>>>>>>
>>>>>>>>> More information can be found on buildroot's patchwork here:
>>>>>>>>> - https://patchwork.ozlabs.org/patch/1092227
>>>>>>>>> - https://patchwork.ozlabs.org/patch/1093385
>>>>>>>>>>
>>>>>>>>>> Jan
>>>>>>>>>
>>>>>>>>> Best Regards,
>>>>>>>>>
>>>>>>>>> Fabrice
>>>>>>>>>
>>>>>>>>
>>>>>>>> So what is your suggestion to fix the O2-regression best?
>>>>>>> I'm far from being an expert in uboot so please excuse me if my answer
>>>>>>> contains some mistakes.
>>>>>>> However, IMHO, we should not pass -O2 or any other optimization flags.
>>>>>>>     From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
>>>>>>> for the target through cross-tools) will (or should?) contain -O2 or
>>>>>>> -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
>>>>>>> However, I do understand that this is a change in the former behavior
>>>>>>> that was always passing -O2 in HOSTCFLAGS.
>>>>>>> So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
>>>>>>> tools/Makefile (after the override).
>>>>>>
>>>>>> This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
>>>>>> setting on its own - which it did prior to your change. I think top-level
>>>>>> HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
>>>>>> appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
>>>>>>
>>>>>>> However, I have to confess that I don't understand why the build
>>>>>>> breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
>>>>>>
>>>>>> IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
>>>>>> all code from tools/image-host.c which is not reachable then. When it's not
>>>>>> removed because of default optimization settings, we get linker errors because
>>>>>> common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
>>>>>> was preferred over #ifdef'ery, but it's broken now.
>>>>> Thanks a lot for your explanation.
>>>>> I was able to reproduce the issue and I think that
>>>>> https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
>>>>> should fixed it.
>>>>> Can you review it? I can also directly send a PR.
>>>>
>>>> I still think that dropping the content of HOSTCFLAGS is the actual bug. There
>>>> is more than -O2 that you lose. If you want your CFLAGS to be respected, append
>>>> them.
>>> I add Arnout to the list to get his feedback as he is the one that
>>> suggested this solution during review of
>>> https://patchwork.ozlabs.org/patch/1092227.
>>
>> Nothing happened, bug is still present in master. Any suggestions how to
>> resolve it?
>
> OK, I've looked over the thread again.  Where and how are you hitting a
> problem here again?  This area is indeed a bit funky and I see
> OpenEmbedded has long been doing "shove everything we need in via CC, do
> not use CFLAGS/etc".  Thanks!

This is how you can easily reproduce it:

make avnet_ultra96_rev1_defconfig
make CROSS_COMPILE=aarch64-linux-gnu- CROSS_BUILD_TOOLS=y

Jan

[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Tom Rini]

On Thu, Nov 14, 2019 at 05:52:36PM +0100, Jan Kiszka wrote:
> On 14.11.19 17:41, Tom Rini wrote:
> > On Thu, Nov 14, 2019 at 05:28:06PM +0100, Jan Kiszka wrote:
> > > On 26.08.19 08:43, Fabrice Fontaine wrote:
> > > > Le lun. 26 août 2019 à 07:57, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > 
> > > > > On 25.08.19 21:11, Fabrice Fontaine wrote:
> > > > > > Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > > > 
> > > > > > > On 25.08.19 17:13, Fabrice Fontaine wrote:
> > > > > > > > Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > > > > > 
> > > > > > > > > On 25.08.19 15:43, Fabrice Fontaine wrote:
> > > > > > > > > > Hello,
> > > > > > > > > > Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka@web.de> a écrit :
> > > > > > > > > > > 
> > > > > > > > > > > On 01.05.19 15:08, Fabrice Fontaine wrote:
> > > > > > > > > > > > When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
> > > > > > > > > > > > will be used with HOSTCFLAGS which seems wrong
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >        tools/Makefile | 1 +
> > > > > > > > > > > >        1 file changed, 1 insertion(+)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/tools/Makefile b/tools/Makefile
> > > > > > > > > > > > index 12a3027e23..eadeba417d 100644
> > > > > > > > > > > > --- a/tools/Makefile
> > > > > > > > > > > > +++ b/tools/Makefile
> > > > > > > > > > > > @@ -272,6 +272,7 @@ subdir- += env
> > > > > > > > > > > > 
> > > > > > > > > > > >        ifneq ($(CROSS_BUILD_TOOLS),)
> > > > > > > > > > > >        override HOSTCC = $(CC)
> > > > > > > > > > > > +override HOSTCFLAGS = $(CFLAGS)
> > > > > > > > > > > > 
> > > > > > > > > > > >        quiet_cmd_crosstools_strip = STRIP   $^
> > > > > > > > > > > >              cmd_crosstools_strip = $(STRIP) $^; touch $@
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > This eats - among other things - -O2, normally set in /Makefile. And that breaks
> > > > > > > > > > > CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
> > > > > > > > > > > (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
> > > > > > > > > > > simply be reverted (which is what I'm doing locally in order to fix my builds).
> > > > > > > > > > I don't think this patch should be reverted, I sent it to fix a
> > > > > > > > > > cross-compilation build issue with host-openssl.
> > > > > > > > > > 
> > > > > > > > > > Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
> > > > > > > > > > sets HOSTCC=$(CC) but it forgets
> > > > > > > > > > to override HOSTCFLAGS, though, so the tools are built with target
> > > > > > > > > > compiler but host CFLAGS.
> > > > > > > > > > 
> > > > > > > > > > This raises a build failure if host-openssl is built before uboot-tools because
> > > > > > > > > > uboot-tools links with openssl headers from host which depends on pthread.h
> > > > > > > > > > 
> > > > > > > > > > More information can be found on buildroot's patchwork here:
> > > > > > > > > > - https://patchwork.ozlabs.org/patch/1092227
> > > > > > > > > > - https://patchwork.ozlabs.org/patch/1093385
> > > > > > > > > > > 
> > > > > > > > > > > Jan
> > > > > > > > > > 
> > > > > > > > > > Best Regards,
> > > > > > > > > > 
> > > > > > > > > > Fabrice
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > So what is your suggestion to fix the O2-regression best?
> > > > > > > > I'm far from being an expert in uboot so please excuse me if my answer
> > > > > > > > contains some mistakes.
> > > > > > > > However, IMHO, we should not pass -O2 or any other optimization flags.
> > > > > > > >     From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
> > > > > > > > for the target through cross-tools) will (or should?) contain -O2 or
> > > > > > > > -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
> > > > > > > > However, I do understand that this is a change in the former behavior
> > > > > > > > that was always passing -O2 in HOSTCFLAGS.
> > > > > > > > So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
> > > > > > > > tools/Makefile (after the override).
> > > > > > > 
> > > > > > > This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
> > > > > > > setting on its own - which it did prior to your change. I think top-level
> > > > > > > HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
> > > > > > > appending (and, thus, overwriting) settings but you cannot remove mandatory ones.
> > > > > > > 
> > > > > > > > However, I have to confess that I don't understand why the build
> > > > > > > > breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."
> > > > > > > 
> > > > > > > IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
> > > > > > > all code from tools/image-host.c which is not reachable then. When it's not
> > > > > > > removed because of default optimization settings, we get linker errors because
> > > > > > > common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
> > > > > > > was preferred over #ifdef'ery, but it's broken now.
> > > > > > Thanks a lot for your explanation.
> > > > > > I was able to reproduce the issue and I think that
> > > > > > https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
> > > > > > should fixed it.
> > > > > > Can you review it? I can also directly send a PR.
> > > > > 
> > > > > I still think that dropping the content of HOSTCFLAGS is the actual bug. There
> > > > > is more than -O2 that you lose. If you want your CFLAGS to be respected, append
> > > > > them.
> > > > I add Arnout to the list to get his feedback as he is the one that
> > > > suggested this solution during review of
> > > > https://patchwork.ozlabs.org/patch/1092227.
> > > 
> > > Nothing happened, bug is still present in master. Any suggestions how to
> > > resolve it?
> > 
> > OK, I've looked over the thread again.  Where and how are you hitting a
> > problem here again?  This area is indeed a bit funky and I see
> > OpenEmbedded has long been doing "shove everything we need in via CC, do
> > not use CFLAGS/etc".  Thanks!
> 
> This is how you can easily reproduce it:
> 
> make avnet_ultra96_rev1_defconfig
> make CROSS_COMPILE=aarch64-linux-gnu- CROSS_BUILD_TOOLS=y

Ah, OK.  So, I wonder about:
commit 72c69ea8d603fd2448dd1d7c399c4f77b77773b7
Author: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Date:   Wed May 1 15:08:25 2019 +0200

    tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

    When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
    will be used with HOSTCFLAGS which seems wrong

    Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

As while it may seem wrong we consistently use KBUILD_CFLAGS for U-Boot
itself (ala the kernel) and HOSTCFLAGS for the tools, both when we run
them on the build host and when we build them for the target.  So for
example the above commit also breaks CONFIG_TOOLS_DEBUG.  Buildroot may
need to fix the problem by appending to HOSTCFLAGS or similar.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191114/f1b89c9a/attachment.sig>