[U-Boot] [PATCH] Don't inline weak symbols

[U-Boot] [PATCH] Don't inline weak symbols

[Roman Zeyde]

Hi Wolfgang ,

I have noticed that the same error as described here
(http://lists.denx.de/pipermail/u-boot/2009-October/061913.html)
happens when compiling the latest version of U-boot (v2009-11-rc1)
using arm-cortex_a8-linux-gnueabi gcc version 4.4.0
(crosstool-NG-hg_default at 1620_187e06178dcb).

Could you please apply the patch described above to resolve this issue?

Thanks,
 Roman.

[U-Boot] [PATCH] Don't inline weak symbols

[Wolfgang Denk]

Dear Roman Zeyde,

In message <6d23e4ef0911030355j47a653a0iea7b6cae2f3cb180@mail.gmail.com> you wrote:
> 
> I have noticed that the same error as described here
> (http://lists.denx.de/pipermail/u-boot/2009-October/061913.html)
> happens when compiling the latest version of U-boot (v2009-11-rc1)
> using arm-cortex_a8-linux-gnueabi gcc version 4.4.0
> (crosstool-NG-hg_default at 1620_187e06178dcb).

This has been discussed here before. Please see the archives.

> Could you please apply the patch described above to resolve this issue?

No, I will not. This has to go through the ARM custodian first.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The perversity of nature is nowhere better demonstrated by  the  fact
that,  when  exposed to the same atmosphere, bread becomes hard while
crackers become soft.

[U-Boot] [PATCH] Don't inline weak symbols

[Tom]

Wolfgang Denk wrote:
> Dear Roman Zeyde,
> 
> In message <6d23e4ef0911030355j47a653a0iea7b6cae2f3cb180@mail.gmail.com> you wrote:
>> I have noticed that the same error as described here
>> (http://lists.denx.de/pipermail/u-boot/2009-October/061913.html)
>> happens when compiling the latest version of U-boot (v2009-11-rc1)
>> using arm-cortex_a8-linux-gnueabi gcc version 4.4.0
>> (crosstool-NG-hg_default at 1620_187e06178dcb).
> 
> This has been discussed here before. Please see the archives.
> 
>> Could you please apply the patch described above to resolve this issue?
> 
> No, I will not. This has to go through the ARM custodian first.
> 
> Best regards,
> 
> Wolfgang Denk
> 

I am currently reviewing this patch.

After reviewing the history.
There is a interest in #if-defing this code.

The reason I can see is at least 2 of the start.S call

	bl	coloured_LED_init
	bl	red_LED_on

In general, what happens is they call into the weak
stubs.  Only if the cpu/board provides an overriding
function does these calls cause anything meaningful to
happen.

A good candidate for an if-def variable is the existing
CONFIG_STATUS_LED.

I am testing this change now.
Since it effects all the boards, I am using the regressions
of undefined symbol from MAKEALL arm to verify.

When this change is done there will only be a handful of
failures for the inline weak problem.

Then this patch to remove the inlines will take care of them.

I have looked at the assembly these produce and it does not
look different for non-inline to inline.  There was a slight
increase in the binary size (zoom2) of 8 bytes using
the code sourcry 2009.  For my roll-your-own gcc 4.4.2, the
size was about 400 bytes larger.  Of course it is harder to
tell since it wouldn't compile without the fix.

After I run through some more testing, I will send out
the patch set.

Tom

[U-Boot] [PATCH] Don't inline weak symbols

[Ron]

GCC 4.4 complains about this now.

Signed-off-by: Ron Lee <ron@debian.org>
---
 lib_arm/board.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib_arm/board.c b/lib_arm/board.c
index a44d308..02a55eb 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -124,23 +124,23 @@ void *sbrk (ptrdiff_t increment)
  * May be supplied by boards if desired
  */
 void inline __coloured_LED_init (void) {}
-void inline coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init")));
+void coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init")));
 void inline __red_LED_on (void) {}
-void inline red_LED_on (void) __attribute__((weak, alias("__red_LED_on")));
+void red_LED_on (void) __attribute__((weak, alias("__red_LED_on")));
 void inline __red_LED_off(void) {}
-void inline red_LED_off(void)	     __attribute__((weak, alias("__red_LED_off")));
+void red_LED_off(void) __attribute__((weak, alias("__red_LED_off")));
 void inline __green_LED_on(void) {}
-void inline green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
+void green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
 void inline __green_LED_off(void) {}
-void inline green_LED_off(void)__attribute__((weak, alias("__green_LED_off")));
+void green_LED_off(void) __attribute__((weak, alias("__green_LED_off")));
 void inline __yellow_LED_on(void) {}
-void inline yellow_LED_on(void)__attribute__((weak, alias("__yellow_LED_on")));
+void yellow_LED_on(void) __attribute__((weak, alias("__yellow_LED_on")));
 void inline __yellow_LED_off(void) {}
-void inline yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off")));
+void yellow_LED_off(void) __attribute__((weak, alias("__yellow_LED_off")));
 void inline __blue_LED_on(void) {}
-void inline blue_LED_on(void)__attribute__((weak, alias("__blue_LED_on")));
+void blue_LED_on(void) __attribute__((weak, alias("__blue_LED_on")));
 void inline __blue_LED_off(void) {}
-void inline blue_LED_off(void)__attribute__((weak, alias("__blue_LED_off")));
+void blue_LED_off(void) __attribute__((weak, alias("__blue_LED_off")));
 
 /************************************************************************
  * Init Utilities							*
-- 
1.6.3.3

[U-Boot] [PATCH] Don't inline weak symbols

[Jean-Christophe PLAGNIOL-VILLARD]

On 03:44 Thu 06 Aug     , Ron wrote:
> 
> GCC 4.4 complains about this now.
I'd remove the inline ot all

Best Regards,
J.

[U-Boot] [PATCH] Don't inline weak symbols

[Ron]

On Wed, Aug 05, 2009 at 11:07:35PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 03:44 Thu 06 Aug     , Ron wrote:
> > 
> > GCC 4.4 complains about this now.
> I'd remove the inline ot all

The compiler likely will already inline/eradicate the defaults there, since
the empty definition is visible to it, so yes, that would be valid too, and
might make that block a bit easier on the eyes :)

Cheers,
Ron

[U-Boot] [PATCH] Don't inline weak symbols

[Wolfgang Denk]

Dear Tom,

In message <20090805181400.GG11898@audi.shelbyville.oz> Ron Lee wrote:
> 
> GCC 4.4 complains about this now.
> 
> Signed-off-by: Ron Lee <ron@debian.org>
> ---
>  lib_arm/board.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/lib_arm/board.c b/lib_arm/board.c
> index a44d308..02a55eb 100644
> --- a/lib_arm/board.c
> +++ b/lib_arm/board.c
> @@ -124,23 +124,23 @@ void *sbrk (ptrdiff_t increment)
>   * May be supplied by boards if desired
>   */
>  void inline __coloured_LED_init (void) {}
> -void inline coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init")));
> +void coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init")));
>  void inline __red_LED_on (void) {}
> -void inline red_LED_on (void) __attribute__((weak, alias("__red_LED_on")));
> +void red_LED_on (void) __attribute__((weak, alias("__red_LED_on")));
>  void inline __red_LED_off(void) {}
> -void inline red_LED_off(void)	     __attribute__((weak, alias("__red_LED_off")));
> +void red_LED_off(void) __attribute__((weak, alias("__red_LED_off")));
>  void inline __green_LED_on(void) {}
> -void inline green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
> +void green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
>  void inline __green_LED_off(void) {}
> -void inline green_LED_off(void)__attribute__((weak, alias("__green_LED_off")));
> +void green_LED_off(void) __attribute__((weak, alias("__green_LED_off")));
>  void inline __yellow_LED_on(void) {}
> -void inline yellow_LED_on(void)__attribute__((weak, alias("__yellow_LED_on")));
> +void yellow_LED_on(void) __attribute__((weak, alias("__yellow_LED_on")));
>  void inline __yellow_LED_off(void) {}
> -void inline yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off")));
> +void yellow_LED_off(void) __attribute__((weak, alias("__yellow_LED_off")));
>  void inline __blue_LED_on(void) {}
> -void inline blue_LED_on(void)__attribute__((weak, alias("__blue_LED_on")));
> +void blue_LED_on(void) __attribute__((weak, alias("__blue_LED_on")));
>  void inline __blue_LED_off(void) {}
> -void inline blue_LED_off(void)__attribute__((weak, alias("__blue_LED_off")));
> +void blue_LED_off(void) __attribute__((weak, alias("__blue_LED_off")));
>  
>  /************************************************************************
>   * Init Utilities							*
> -- 

Do you have this patch on your list?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Humans do claim a great deal for that particular emotion (love).
	-- Spock, "The Lights of Zetar", stardate 5725.6