[U-Boot] [RFC 00/10] clang support for ARM
Hi, I rebased some patches with should compile a working u-boot with clang and gcc (besides a couple which don't have memset), but I didn't bother too much about that for now.
Obviously this is not intended for the next release, but a request for comment. Clang is not really fond of the gd assignment and I tried to get rid of them since crt0.S already handles this. I also added clearing gd (in a broken way) to crt0.S to prevent all board_init_f having to do it again and again (i2c / serial typically depends on it).
The most important question is if these patches break any existing board. Your feedback is welcome.
Kind regards, Jeroen
patches are also at (with some twister board specific ones): https://github.com/jhofstee/u-boot/commits/v2014.07-rc2-clang
Jeroen Hofstee (10): ARM: crt0.S: clear the global data ARM: omap3/board: add spl specific board_init_f board_r: only assign gd when requested ARM: do not set gd in generic board again ARM: SPL: do not set gd again cc-option: make it work with clang ARM: make gd a function a function for clang inline: use the gcc inline version instead of the c99 one. eabi_compat: add __aeabi_memcpy __aeabi_memset README.clang: build command with clang
arch/arm/cpu/armv7/omap3/board.c | 15 ++++++++++----- arch/arm/include/asm/config.h | 2 -- arch/arm/include/asm/global_data.h | 17 +++++++++++++++++ arch/arm/lib/crt0.S | 6 ++++++ arch/arm/lib/eabi_compat.c | 15 +++++++++++++-- arch/arm/lib/spl.c | 3 --- common/board_r.c | 2 +- doc/README.clang | 15 +++++++++++++++ include/linux/compiler-gcc.h | 7 ++++--- scripts/Kbuild.include | 4 ++-- 10 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 doc/README.clang
NOTE: smdk5420 snow smdkv310 apf27 arndale origen vpac270_ond_256 smdk5250 don't have a memset available. --- arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index dfc2de9..52b6f74 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -70,6 +70,12 @@ ENTRY(_main) sub sp, sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r9, sp /* GD is above SP */ + + mov r0, r9 /* Clear the global data */ + mov r1, #0 + mov r2, #GENERATED_GBL_DATA_SIZE + bl memset + mov r0, #0 bl board_init_f
Hi Jeroen,
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
NOTE: smdk5420 snow smdkv310 apf27 arndale origen vpac270_ond_256 smdk5250 don't have a memset available.
You should add a message explaining the motivation for the change.
Can you add a memset()?
Regards, Simon
Hello Simon,
On ma, 2014-06-02 at 20:02 -0600, Simon Glass wrote:
Hi Jeroen,
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
NOTE: smdk5420 snow smdkv310 apf27 arndale origen vpac270_ond_256 smdk5250 don't have a memset available.
You should add a message explaining the motivation for the change.
yes, I will actually check all commit messages, fix the typos and add proper tags and cc's etc. Make the documentation a bit more generic and add some comment about why gd needs to be treated special for llvm in the first place.
That said, regarding this patch, the reason to clear gd at all is that e.g. i2c depends on it and there have been issues with the serial console flags etc as well. The reason to do it directly after allocating gd is that there are several board_init_f's with the more wide spread use of SPL. Another problem is that some code attempts to store values in global data assuming it will survive relocation, but failed since gd got reassigned again.
When just "allocating" global data once and clearing it directly, such problem should be gone. It also matches the behavior of the generic board CONFIG_SYS_GENERIC_GLOBAL_DATA, so this actually preserves current behavior for the ARM boards using the generic board after the removal of CONFIG_SYS_GENERIC_GLOBAL_DATA.
Can you add a memset()?
Likely, one way or the other, it will be easily to fix. I posted it as is since I was wondering what Albert thinks about it. (for completeness a final version will include a similar change for aarch64).
Regards, Jeroen
On Sat, May 31, 2014 at 1:32 PM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
NOTE: smdk5420 snow smdkv310 apf27 arndale origen vpac270_ond_256 smdk5250 don't have a memset available.
arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index dfc2de9..52b6f74 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -70,6 +70,12 @@ ENTRY(_main) sub sp, sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r9, sp /* GD is above SP */
mov r0, r9 /* Clear the global data */mov r1, #0mov r2, #GENERATED_GBL_DATA_SIZEbl memsetmov r0, #0 bl board_init_f--
Acked-by: Tim Harvey tharvey@gateworks.com
This resolves the issue I encountered and reported with IMX6 SPL.
Thanks,
Tim
Hi,
On 31-05-14 22:32, Jeroen Hofstee wrote:
NOTE: smdk5420 snow smdkv310 apf27 arndale origen vpac270_ond_256 smdk5250 don't have a memset available.
arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index dfc2de9..52b6f74 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -70,6 +70,12 @@ ENTRY(_main) sub sp, sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r9, sp /* GD is above SP */
- mov r0, r9 /* Clear the global data */
- mov r1, #0
- mov r2, #GENERATED_GBL_DATA_SIZE
- bl memset
- mov r0, #0 bl board_init_f
this patch is superseded by http://patchwork.ozlabs.org/patch/368944/.
Regards, Jeroen
gd is already setup when calling board_init_f and preloader_console_init depends on it. Therefore move the SPL specific part to its own board_init_f as it prevents the need for a temporarily gd. --- arch/arm/cpu/armv7/omap3/board.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index 9bb1a1c..2e2ec14 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -239,17 +239,22 @@ void s_init(void) ehci_clocks_enable(); #endif
+ if (!in_sdram) + mem_init(); +} + #ifdef CONFIG_SPL_BUILD - gd = &gdata; +void board_init_f(ulong dummy) +{ + /* Clear the BSS. */ + memset(__bss_start, 0, __bss_end - __bss_start);
preloader_console_init(); - timer_init(); -#endif
- if (!in_sdram) - mem_init(); + board_init_r(NULL, 0); } +#endif
/* * Routine: misc_init_r
When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles the assignment of gd. At least in case of ARM/Aarch64 this means board_init_r is alteady called with the new gd. Therefore only assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined. --- common/board_r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_r.c b/common/board_r.c index 602a239..18bbe26 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) int i; #endif
-#ifndef CONFIG_X86 +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA gd = new_gd; #endif
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles the assignment of gd. At least in case of ARM/Aarch64 this means board_init_r is alteady called with the new gd. Therefore only assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
I think you might be missing a signoff (you might consider using patman). Apart from that it looks good.
Acked-by: Simon Gass sjg@chromium.org
common/board_r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_r.c b/common/board_r.c index 602a239..18bbe26 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) int i; #endif
-#ifndef CONFIG_X86 +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA gd = new_gd; #endif
-- 1.8.3.2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hello Wolfgang / Stefan.
On za, 2014-05-31 at 22:32 +0200, Jeroen Hofstee wrote:
When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles the assignment of gd. At least in case of ARM/Aarch64 this means board_init_r is alteady called with the new gd. Therefore only assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
common/board_r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_r.c b/common/board_r.c index 602a239..18bbe26 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) int i; #endif
-#ifndef CONFIG_X86 +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA gd = new_gd; #endif
Can any of you confirm this change is fine for powerpc as well?
Regards, Jeroen
Hi Jeroen,
(added York to cc as he introduced CONFIG_SYS_GENERIC_GLOBAL_DATA with patch 2a1680e3 [common/board_f: Initialized global data for generic board])
On 03.06.2014 22:52, Jeroen Hofstee wrote:
Hello Wolfgang / Stefan.
On za, 2014-05-31 at 22:32 +0200, Jeroen Hofstee wrote:
When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles the assignment of gd. At least in case of ARM/Aarch64 this means board_init_r is alteady called with the new gd. Therefore only assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
common/board_r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_r.c b/common/board_r.c index 602a239..18bbe26 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) int i; #endif
-#ifndef CONFIG_X86 +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA gd = new_gd; #endif
Can any of you confirm this change is fine for powerpc as well?
powerpc doesn't define CONFIG_SYS_GENERIC_GLOBAL_DATA right now. And I'm not sure which powerpc boards use the common board_f/_r functions right now. Perhaps York knows?
Other than that I don't any problems, so:
Acked-by: Stefan Roese sr@denx.de
Thanks, Stefan
On Jun 3, 2014, at 8:52 PM, Stefan Roese wrote:
Hi Jeroen,
(added York to cc as he introduced CONFIG_SYS_GENERIC_GLOBAL_DATA with patch 2a1680e3 [common/board_f: Initialized global data for generic board])
On 03.06.2014 22:52, Jeroen Hofstee wrote:
Hello Wolfgang / Stefan.
On za, 2014-05-31 at 22:32 +0200, Jeroen Hofstee wrote:
When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles the assignment of gd. At least in case of ARM/Aarch64 this means board_init_r is alteady called with the new gd. Therefore only assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
common/board_r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_r.c b/common/board_r.c index 602a239..18bbe26 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) int i; #endif
-#ifndef CONFIG_X86 +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA gd = new_gd; #endif
Can any of you confirm this change is fine for powerpc as well?
powerpc doesn't define CONFIG_SYS_GENERIC_GLOBAL_DATA right now. And I'm not sure which powerpc boards use the common board_f/_r functions right now. Perhaps York knows?
I think this change is not right for powerpc. The idea of using CONFIG_SYS_GENERIC_GLOBAL_DATA is to use the stack for gd in board_init_f. PowerPC boards don't use this way. The gd is initialized and used before calling board_init_f.
PowerPC boards also use new_gd when calling board_init_r. If you add this macro, I don't think powerpc will continue to work.
This commit 15672c6dbd7e5a110773480ccfe47b98ba1dc6f8 converts some powerpc boards to use generic board.
York
Since crt0.S / crt0_64.S already allocate gd and assing it there is no need to do it again board_f / board_r as it only wastes stack space. --- arch/arm/include/asm/config.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h index 2a20a77..abf79e5 100644 --- a/arch/arm/include/asm/config.h +++ b/arch/arm/include/asm/config.h @@ -7,8 +7,6 @@ #ifndef _ASM_CONFIG_H_ #define _ASM_CONFIG_H_
-#define CONFIG_SYS_GENERIC_GLOBAL_DATA - #define CONFIG_LMB #define CONFIG_SYS_BOOT_RAMDISK_HIGH
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Since crt0.S / crt0_64.S already allocate gd and assing it there is no need to do it again board_f / board_r as it only wastes stack space.
Acked-by: Simon Gass sjg@chromium.org
arch/arm/include/asm/config.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h index 2a20a77..abf79e5 100644 --- a/arch/arm/include/asm/config.h +++ b/arch/arm/include/asm/config.h @@ -7,8 +7,6 @@ #ifndef _ASM_CONFIG_H_ #define _ASM_CONFIG_H_
-#define CONFIG_SYS_GENERIC_GLOBAL_DATA
#define CONFIG_LMB #define CONFIG_SYS_BOOT_RAMDISK_HIGH
-- 1.8.3.2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
--- arch/arm/lib/spl.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c index dfcc596..75ab546 100644 --- a/arch/arm/lib/spl.c +++ b/arch/arm/lib/spl.c @@ -28,9 +28,6 @@ void __weak board_init_f(ulong dummy) /* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start);
- /* Set global data pointer. */ - gd = &gdata; - board_init_r(NULL, 0); }
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
arch/arm/lib/spl.c | 3 --- 1 file changed, 3 deletions(-)
Needs commit message/signoff, but this seems to be throughout the series. Apart from that:
Acked-by: Simon Gass sjg@chromium.org
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c index dfcc596..75ab546 100644 --- a/arch/arm/lib/spl.c +++ b/arch/arm/lib/spl.c @@ -28,9 +28,6 @@ void __weak board_init_f(ulong dummy) /* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start);
/* Set global data pointer. */gd = &gdata;board_init_r(NULL, 0);}
-- 1.8.3.2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Sat, May 31, 2014 at 1:32 PM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
arch/arm/lib/spl.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c index dfcc596..75ab546 100644 --- a/arch/arm/lib/spl.c +++ b/arch/arm/lib/spl.c @@ -28,9 +28,6 @@ void __weak board_init_f(ulong dummy) /* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start);
/* Set global data pointer. */gd = &gdata;board_init_r(NULL, 0);}
Agree with Simon's comment
otherwise
Acked-by: Tim Harvey tharvey@gateworks.com
By default clang will return echo a warning if an option is unknown. Therefore turn warnings into errors when polling for options. --- scripts/Kbuild.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index c664e39..4c33359 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -113,12 +113,12 @@ as-instr = $(call try-run,\ # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
cc-option = $(call try-run,\ - $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2)) + $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
# cc-option-yn # Usage: flag := $(call cc-option-yn,-march=winchip-c6) cc-option-yn = $(call try-run,\ - $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n) + $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n)
# cc-option-align # Prefix align with either -falign or -malign
Hi Jeroen,
On Sat, 31 May 2014 22:32:20 +0200 Jeroen Hofstee jeroen@myspectrum.nl wrote:
By default clang will return echo a warning if an option is unknown. Therefore turn warnings into errors when polling for options.
As far as I tested with clang 3.5 on Ubuntu 14.04, it looks different.
$ clang -fno-delete-null-pointer-checks helloworld.c clang: error: unknown argument: '-fno-delete-null-pointer-checks' $ echo $? 1
Best Regards Masahiro Yamada
Hi Masahiro,
On Tue, 10 Jun 2014 17:39:03 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Jeroen,
On Sat, 31 May 2014 22:32:20 +0200 Jeroen Hofstee jeroen@myspectrum.nl wrote:
By default clang will return echo a warning if an option is unknown. Therefore turn warnings into errors when polling for options.
As far as I tested with clang 3.5 on Ubuntu 14.04, it looks different.
$ clang -fno-delete-null-pointer-checks helloworld.c clang: error: unknown argument: '-fno-delete-null-pointer-checks' $ echo $? 1
Possibly it depends on how the clang/LLVM toolchain in use has been built and configured. In any case, explicitly turning warnings into errors fixes those toolchains which only emit warnings for unknown options, and it won't harm those such as the one above which already do "the right thing".
Best Regards Masahiro Yamada
Amicalement,
Hello Masahiro,
On 10-06-14 10:39, Masahiro Yamada wrote:
Hi Jeroen,
On Sat, 31 May 2014 22:32:20 +0200 Jeroen Hofstee jeroen@myspectrum.nl wrote:
By default clang will return echo a warning if an option is unknown. Therefore turn warnings into errors when polling for options.
As far as I tested with clang 3.5 on Ubuntu 14.04, it looks different.
$ clang -fno-delete-null-pointer-checks helloworld.c clang: error: unknown argument: '-fno-delete-null-pointer-checks' $ echo $? 1
Yes it seems to be restricted to warning options. The -Werror=date-time causes a lot of noise e.g. when building with clang 3.4. With 3.5 this option seems to be added, but behavior for unrecognized warnings flags remained the same, see below.
Regards, Jeroen
[jeroen@freebsd /usr/home/jeroen]$ clang -v FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512 Target: x86_64-unknown-freebsd11.0 Thread model: posix Selected GCC installation: [jeroen@freebsd /usr/home/jeroen]$ clang -Werror=date-time a.c warning: unknown warning option '-Werror=date-time' [-Wunknown-warning-option] 1 warning generated. [jeroen@freebsd /usr/home/jeroen]$ echo $? 0 [jeroen@freebsd /usr/home/jeroen]$ clang -Werror -Werror=date-time a.c error: unknown warning option '-Werror=date-time' [-Werror,-Wunknown-warning-option] [jeroen@freebsd /usr/home/jeroen]$ echo $? 1
jeroen@yellow:~$ clang -v clang version 3.5.0 (git@github.com:jhofstee/clang.git f533fd477a50467a0d96293d116f4059aa806b65) (git@github.com:jhofstee/llvm.git 6b7ff6be9c1bcf8ce440c7f1c7646fbf059562e4) Target: x86_64-unknown-linux-gnu Thread model: posix Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/4.8 Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/4.8.1 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8.1 Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8 Candidate multilib: .;@m64 Selected multilib: .;@m64 jeroen@yellow:~$ clang -Werror=date-time a.c jeroen@yellow:~$ clang -Werror=date-time-newer a.c warning: unknown warning option '-Werror=date-time-newer'; did you mean '-Werror=date-time'? [-Wunknown-warning-option] 1 warning generated. jeroen@yellow:~$ echo $? 0 jeroen@yellow:~$ clang -Werror -Werror=date-time-newer a.c error: unknown warning option '-Werror=date-time-newer'; did you mean '-Werror=date-time'? [-Werror,-Wunknown-warning-option] jeroen@yellow:~$ echo $? 1
--- arch/arm/include/asm/global_data.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 63e4ad5..646d694 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -44,10 +44,27 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
+#ifdef __clang__ + +#define DECLARE_GLOBAL_DATA_PTR +#define gd get_gd() + +static __inline volatile gd_t *get_gd(void) +{ + gd_t *gd_ptr; + + __asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr)); + + return gd_ptr; +} + +#else + #ifdef CONFIG_ARM64 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("x18") #else #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9") #endif +#endif
#endif /* __ASM_GBL_DATA_H */
Hi Jeroen,
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
arch/arm/include/asm/global_data.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 63e4ad5..646d694 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -44,10 +44,27 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
+#ifdef __clang__
+#define DECLARE_GLOBAL_DATA_PTR +#define gd get_gd()
+static __inline volatile gd_t *get_gd(void) +{
gd_t *gd_ptr;__asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr));return gd_ptr;+}
+#else
#ifdef CONFIG_ARM64 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("x18") #else #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9") #endif +#endif
#endif /* __ASM_GBL_DATA_H */
Probably a good idea to copy Albert on these patches. What happens if you compile ARM64 with Clang?
Regards, Simon
On ma, 2014-06-02 at 20:20 -0600, Simon Glass wrote:
Hi Jeroen,
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
arch/arm/include/asm/global_data.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 63e4ad5..646d694 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -44,10 +44,27 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
+#ifdef __clang__
+#define DECLARE_GLOBAL_DATA_PTR +#define gd get_gd()
+static __inline volatile gd_t *get_gd(void) +{
gd_t *gd_ptr;__asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr));return gd_ptr;+}
+#else
#ifdef CONFIG_ARM64 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("x18") #else #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9") #endif +#endif
#endif /* __ASM_GBL_DATA_H */
Probably a good idea to copy Albert on these patches. What happens if you compile ARM64 with Clang?
Regards, Simon
Hi Simon (this time with reply..)
On ma, 2014-06-02 at 20:20 -0600, Simon Glass wrote:
Hi Jeroen,
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
arch/arm/include/asm/global_data.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 63e4ad5..646d694 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -44,10 +44,27 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
+#ifdef __clang__
+#define DECLARE_GLOBAL_DATA_PTR +#define gd get_gd()
+static __inline volatile gd_t *get_gd(void) +{
gd_t *gd_ptr;__asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr));return gd_ptr;+}
+#else
#ifdef CONFIG_ARM64 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("x18") #else #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9") #endif +#endif
#endif /* __ASM_GBL_DATA_H */
Probably a good idea to copy Albert on these patches. What happens if you compile ARM64 with Clang?
As is it will complain that is has no idea what r9 is. At the moment clang does not have a -ffixed-x18 so there is no polite way to make it work. With a slightly hacked llvm it seems promising (it compiles at least and disassembly seems fine). I have no hardware to test it on though. It would be nice to know if it actually boots..
Perhaps the best thing to do for now is error out and point to the README explaining why it doesn't work.
Regards, Jeroen
Hi Jeroen,
On 3 June 2014 13:58, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Hi Simon (this time with reply..)
On ma, 2014-06-02 at 20:20 -0600, Simon Glass wrote:
Hi Jeroen,
On 31 May 2014 14:32, Jeroen Hofstee jeroen@myspectrum.nl wrote:
arch/arm/include/asm/global_data.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 63e4ad5..646d694 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -44,10 +44,27 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
+#ifdef __clang__
+#define DECLARE_GLOBAL_DATA_PTR +#define gd get_gd()
+static __inline volatile gd_t *get_gd(void) +{
gd_t *gd_ptr;__asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr));return gd_ptr;+}
+#else
#ifdef CONFIG_ARM64 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("x18") #else #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9") #endif +#endif
#endif /* __ASM_GBL_DATA_H */
Probably a good idea to copy Albert on these patches. What happens if you compile ARM64 with Clang?
As is it will complain that is has no idea what r9 is. At the moment clang does not have a -ffixed-x18 so there is no polite way to make it work. With a slightly hacked llvm it seems promising (it compiles at least and disassembly seems fine). I have no hardware to test it on though. It would be nice to know if it actually boots..
Perhaps the best thing to do for now is error out and point to the README explaining why it doesn't work.
You make it no worse by implementing it with a warning comment in the code. You could even use #warning perrhaps.
Regards, Simon
--- This fixes errors like:
make[1]: Entering directory `/home/jeroen/software/u-boot/arch/arm/cpu/armv7/omap-common' arm-linux-gnueabi-ld.bfd -r -o libomap-common.o reset.o timer.o utils.o timer.o: In function `get_tbclk': /home/jeroen/software/u-boot/include/asm/io.h:81: multiple definition of `__raw_writesb' reset.o:/home/jeroen/software/u-boot/include/asm/io.h:81: first defined here timer.o: In function `__udelay': /home/jeroen/software/u-boot/include/asm/io.h:88: multiple definition of `__raw_writesw' reset.o:/home/jeroen/software/u-boot/include/asm/io.h:88: first defined here timer.o: In function `get_ticks': /home/jeroen/software/u-boot/include/asm/io.h:95: multiple definition of `__raw_writesl' reset.o:/home/jeroen/software/u-boot/include/asm/io.h:95: first defined here timer.o: In function `__raw_readsb': /home/jeroen/software/u-boot/include/asm/io.h:102: multiple definition of `__raw_readsb' reset.o:/home/jeroen/software/u-boot/include/asm/io.h:102: first defined here timer.o: In function `__raw_readsw': /home/jeroen/software/u-boot/include/asm/io.h:109: multiple definition of `__raw_readsw' reset.o:/home/jeroen/software/u-boot/include/asm/io.h:109: first defined here timer.o: In function `__raw_readsl': /home/jeroen/software/u-boot/include/asm/io.h:116: multiple definition of `__raw_readsl' reset.o:/home/jeroen/software/u-boot/include/asm/io.h:116: first defined here make[1]: *** [libomap-common.o] Error 1 make[1]: Leaving directory `/home/jeroen/software/u-boot/arch/arm/cpu/armv7/omap-common' make: *** [arch/arm/cpu/armv7/omap-common/libomap-common.o] Error 2 --- include/linux/compiler-gcc.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index 9896e54..99c6dcc 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -44,9 +44,10 @@ */ #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \ !defined(CONFIG_OPTIMIZE_INLINING) || (__GNUC__ < 4) -# define inline inline __attribute__((always_inline)) -# define __inline__ __inline__ __attribute__((always_inline)) -# define __inline __inline __attribute__((always_inline)) +/* XXX: check __GNUC_STDC_INLINE__, fix line length */ +# define inline inline __attribute__((always_inline)) __attribute__((__gnu_inline__)) +# define __inline__ __inline__ __attribute__((always_inline)) __attribute__((__gnu_inline__)) +# define __inline __inline __attribute__((always_inline)) __attribute__((__gnu_inline__)) #endif
#define __deprecated __attribute__((deprecated))
fixes this:
common/libcommon.o: In function `env_callback_init': /home/jeroen/software/u-boot/common/env_callback.c:47: undefined reference to `__aeabi_memset' common/libcommon.o: In function `parse_stream_outer': /home/jeroen/software/u-boot/common/hush.c:3154: undefined reference to `__aeabi_memset' disk/libdisk.o: In function `get_device_and_partition': /home/jeroen/software/u-boot/disk/part.c:596: undefined reference to `__aeabi_memcpy' /home/jeroen/software/u-boot/disk/part.c:605: undefined reference to `__aeabi_memcpy' fs/fat/libfat.o: In function `fat_set_blk_dev': /home/jeroen/software/u-boot/fs/fat/fat.c:60: undefined reference to `__aeabi_memcpy' fs/ubifs/libubifs.o: In function `ubifs_tnc_locate': /home/jeroen/software/u-boot/fs/ubifs/tnc.c:1457: undefined reference to `__aeabi_memcpy' fs/ubifs/libubifs.o: In function `tnc_insert': /home/jeroen/software/u-boot/fs/ubifs/tnc.c:2008: undefined reference to `__aeabi_memcpy' fs/ubifs/libubifs.o:/home/jeroen/software/u-boot/fs/ubifs/tnc.c:2333: more undefined references to `__aeabi_memcpy' follow arm-linux-gnueabi-ld.bfd: BFD (GNU Binutils for Ubuntu) 2.23.2 assertion fail ../../bfd/elf32-arm.c:7683 arm-linux-gnueabi-ld.bfd: BFD (GNU Binutils for Ubuntu) 2.23.2 assertion fail ../../bfd/elf32-arm.c:7683 arm-linux-gnueabi-ld.bfd: error: required section '.rel.plt' not found in the linker script arm-linux-gnueabi-ld.bfd: final link failed: Invalid operation --- arch/arm/lib/eabi_compat.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c index 10d1933..a2cb06e 100644 --- a/arch/arm/lib/eabi_compat.c +++ b/arch/arm/lib/eabi_compat.c @@ -20,8 +20,19 @@ int raise (int signum) /* Dummy function to avoid linker complaints */ void __aeabi_unwind_cpp_pr0(void) { -}; +}
void __aeabi_unwind_cpp_pr1(void) { -}; +} + +/* Copy memory like memcpy, but no return value required. */ +void __aeabi_memcpy(void *dest, const void *src, size_t n) +{ + (void) memcpy(dest, src, n); +} + +void __aeabi_memset(void *dest, size_t n, int c) +{ + (void) memset(dest, c, n); +}
--- doc/README.clang | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 doc/README.clang
diff --git a/doc/README.clang b/doc/README.clang new file mode 100644 index 0000000..59d39a4 --- /dev/null +++ b/doc/README.clang @@ -0,0 +1,15 @@ +NOTE: target compilation only work for (some) ARM boards +at the moment. Also Aarch64 is not supported. + +The Debian clang package does not have ARM support. It must be +compiled manually. + +To compile U-Boot with clang use e.g.: +export CROSS_COMPILE=arm-linux-gnueabi-; make HOSTCC=clang CC="clang -no-integrated-as -target $CROSS_COMPILE -B/usr/arm-linux-gnueabi/lib/ -mllvm -arm-use-movt=0" V=1 twister_config all -j8 + +If all is well you end up on the target with e.g.: + +ver +U-Boot 2014.07-rc2-00067-gd410009 (May 31 2014 - 20:01:13) +clang version 3.5.0 (https://github.com/llvm-mirror/clang.git 9d0a0d8e196610bb7fef57a5fa034b2f628f870f) (https://github.com/llvm-mirror/llvm.git f2928b9b5f3d2af68f724af16cdaed2628fddfc9) +GNU ld (GNU Binutils for Ubuntu) 2.23.52.20130913
Changes since v1 (RFC): - Update the commit message for -Werror when polling for cc-options. As pointed out by Masahiro this is not needed for all arguments, but only for warning options. - drop SOC specific patch from this patchset, I will post it seperately. - Do implement get_gd for AARCH64. - Drop the memset patch for clearing gd / not reassinging gd and use Simon his version. - Drop the patch for using gnu inline version, since targets using the generic board no longer need it. - Limit update gd in board_init_r to ARM / ARM64, since powerpc does need it. - change __inline to inline and drop volatile from get_gd to make checkpatch happy. - add patch workaround for generated constants - add patch default to cc for host compiler - update README.clang - add SOB / cc
This patchset depends on: http://patchwork.ozlabs.org/patch/368944/ (pending dm pull) http://patchwork.ozlabs.org/patch/362580/
Jeroen Hofstee (8): board_r: ARM[64] do not set gd again ARM: SPL: do not set gd again cc-option: also detect unsupported warnings options ARM: make gd a function for clang eabi_compat: add __aeabi_memcpy __aeabi_memset clang: workaround for generated constants Makefile: default to cc for host compiler README.clang: build command with clang
Kbuild | 3 +- Makefile | 2 +- arch/arm/include/asm/global_data.h | 25 +++++++++++++++++ arch/arm/lib/eabi_compat.c | 15 ++++++++-- arch/arm/lib/spl.c | 3 -- common/board_r.c | 2 +- doc/README.clang | 56 ++++++++++++++++++++++++++++++++++++++ include/linux/kbuild.h | 6 ++-- scripts/Kbuild.include | 4 +-- 9 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 doc/README.clang
For ARM / ARM64 the relocation routines already updated gd to the new value. Don't set it again. This allows compilation with clang as it cannot update gd directly.
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- common/board_r.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_r.c b/common/board_r.c index 8e7a3ac..c01d9cd 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -911,7 +911,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) int i; #endif
-#ifndef CONFIG_X86 +#if !defined(CONFIG_X86) && !defined(CONFIG_ARM) && !defined(CONFIG_ARM64) gd = new_gd; #endif
Just before calling board_init_f, crt0.S has already reserved space for the initial gd on the stack. There should be no need to allocate it again.
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/lib/spl.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c index dfcc596..75ab546 100644 --- a/arch/arm/lib/spl.c +++ b/arch/arm/lib/spl.c @@ -28,9 +28,6 @@ void __weak board_init_f(ulong dummy) /* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start);
- /* Set global data pointer. */ - gd = &gdata; - board_init_r(NULL, 0); }
By default clang will echo a warning if a warning option is unknown. Turning warnings into errors when polling for options also catches such cases and prevents passing arguments to the compiler which cause warnings.
cc: Masahiro Yamada yamada.m@jp.panasonic.com cc: Tom Rini trini@ti.com Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- scripts/Kbuild.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index c664e39..4c33359 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -113,12 +113,12 @@ as-instr = $(call try-run,\ # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
cc-option = $(call try-run,\ - $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2)) + $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
# cc-option-yn # Usage: flag := $(call cc-option-yn,-march=winchip-c6) cc-option-yn = $(call try-run,\ - $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n) + $(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n)
# cc-option-align # Prefix align with either -falign or -malign
"clang does not support global register variables; this is unlikely to be implemented soon because it requires additional LLVM backend support" [1]
Workaround it by obtaining the value of gd/r9 by an inline asm routine. Note there is no set routine added for ARM at the moment, since most if not all updates of gd from c are actually not needed for ARM.
[1] http://clang.llvm.org/docs/UsersManual.html
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/include/asm/global_data.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 63e4ad5..c69d064 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -44,10 +44,35 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
+#ifdef __clang__ + +#define DECLARE_GLOBAL_DATA_PTR +#define gd get_gd() + +static inline gd_t *get_gd(void) +{ + gd_t *gd_ptr; + +#ifdef CONFIG_ARM64 + /* + * Make will already error that reserving x18 is not supported at the + * time of writing, clang: error: unknown argument: '-ffixed-x18' + */ + __asm__ volatile("mov %0, x18\n" : "=r" (gd_ptr)); +#else + __asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr)); +#endif + + return gd_ptr; +} + +#else + #ifdef CONFIG_ARM64 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("x18") #else #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9") #endif +#endif
#endif /* __ASM_GBL_DATA_H */
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- fixes this:
common/libcommon.o: In function `env_callback_init': /home/jeroen/software/u-boot/common/env_callback.c:47: undefined reference to `__aeabi_memset' common/libcommon.o: In function `parse_stream_outer': /home/jeroen/software/u-boot/common/hush.c:3154: undefined reference to `__aeabi_memset' disk/libdisk.o: In function `get_device_and_partition': /home/jeroen/software/u-boot/disk/part.c:596: undefined reference to `__aeabi_memcpy' /home/jeroen/software/u-boot/disk/part.c:605: undefined reference to `__aeabi_memcpy' fs/fat/libfat.o: In function `fat_set_blk_dev': /home/jeroen/software/u-boot/fs/fat/fat.c:60: undefined reference to `__aeabi_memcpy' fs/ubifs/libubifs.o: In function `ubifs_tnc_locate': /home/jeroen/software/u-boot/fs/ubifs/tnc.c:1457: undefined reference to `__aeabi_memcpy' fs/ubifs/libubifs.o: In function `tnc_insert': /home/jeroen/software/u-boot/fs/ubifs/tnc.c:2008: undefined reference to `__aeabi_memcpy' fs/ubifs/libubifs.o:/home/jeroen/software/u-boot/fs/ubifs/tnc.c:2333: more undefined references to `__aeabi_memcpy' follow arm-linux-gnueabi-ld.bfd: BFD (GNU Binutils for Ubuntu) 2.23.2 assertion fail ../../bfd/elf32-arm.c:7683 arm-linux-gnueabi-ld.bfd: BFD (GNU Binutils for Ubuntu) 2.23.2 assertion fail ../../bfd/elf32-arm.c:7683 arm-linux-gnueabi-ld.bfd: error: required section '.rel.plt' not found in the linker script arm-linux-gnueabi-ld.bfd: final link failed: Invalid operation --- arch/arm/lib/eabi_compat.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c index 10d1933..a2cb06e 100644 --- a/arch/arm/lib/eabi_compat.c +++ b/arch/arm/lib/eabi_compat.c @@ -20,8 +20,19 @@ int raise (int signum) /* Dummy function to avoid linker complaints */ void __aeabi_unwind_cpp_pr0(void) { -}; +}
void __aeabi_unwind_cpp_pr1(void) { -}; +} + +/* Copy memory like memcpy, but no return value required. */ +void __aeabi_memcpy(void *dest, const void *src, size_t n) +{ + (void) memcpy(dest, src, n); +} + +void __aeabi_memset(void *dest, size_t n, int c) +{ + (void) memset(dest, c, n); +}
KBuild abuses the asm statement to write to a file and clang chokes about these invalid asm statements. Hack it even more by fooling this is actual valid asm code.
cc: Masahiro Yamada yamada.m@jp.panasonic.com cc: Tom Rini trini@ti.com Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- Kbuild | 3 ++- include/linux/kbuild.h | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/Kbuild b/Kbuild index 6e1698c..ef97787 100644 --- a/Kbuild +++ b/Kbuild @@ -53,7 +53,8 @@ targets += arch/$(ARCH)/lib/asm-offsets.s
# Default sed regexp - multiline due to syntax constraints define sed-y - "/^->/{s:->#(.*):/* \1 */:; \ + "s:[[:space:]]*.ascii[[:space:]]*"(.*)":\1:; \ + /^->/{s:->#(.*):/* \1 */:; \ s:^->([^ ]*) [$$#]*([-0-9]*) (.*):#define \1 \2 /* \3 */:; \ s:^->([^ ]*) [$$#]*([^ ]*) (.*):#define \1 \2 /* \3 */:; \ s:->::; p;}" diff --git a/include/linux/kbuild.h b/include/linux/kbuild.h index ab7805a..8a9f645 100644 --- a/include/linux/kbuild.h +++ b/include/linux/kbuild.h @@ -7,14 +7,14 @@ #define __LINUX_KBUILD_H
#define DEFINE(sym, val) \ - asm volatile("\n->" #sym " %0 " #val : : "i" (val)) + asm volatile("\n.ascii "->" #sym " %0 " #val """ : : "i" (val))
-#define BLANK() asm volatile("\n->" : : ) +#define BLANK() asm volatile("\n.ascii "->"" : : )
#define OFFSET(sym, str, mem) \ DEFINE(sym, offsetof(struct str, mem))
#define COMMENT(x) \ - asm volatile("\n->#" x) + asm volatile("\n.ascii "->#" x """)
#endif
Since the host compiler might not be gcc but e.g. clang default to cc to invoke it.
cc: Masahiro Yamada yamada.m@jp.panasonic.com cc: Tom Rini trini@ti.com Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 80eb239..c19f7d9 100644 --- a/Makefile +++ b/Makefile @@ -204,7 +204,7 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \ else if [ -x /bin/bash ]; then echo /bin/bash; \ else echo sh; fi ; fi)
-HOSTCC = gcc +HOSTCC = cc HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer HOSTCXXFLAGS = -O2
Hi Jeroen,
On Wed, 30 Jul 2014 21:54:55 +0200 Jeroen Hofstee jeroen@myspectrum.nl wrote:
Since the host compiler might not be gcc but e.g. clang default to cc to invoke it.
cc: Masahiro Yamada yamada.m@jp.panasonic.com cc: Tom Rini trini@ti.com Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 80eb239..c19f7d9 100644 --- a/Makefile +++ b/Makefile @@ -204,7 +204,7 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \ else if [ -x /bin/bash ]; then echo /bin/bash; \ else echo sh; fi ; fi)
-HOSTCC = gcc +HOSTCC = cc HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer HOSTCXXFLAGS = -O2
For consistency,
HOSTCXX = c++
?
Kconfig has a C++ host program. (scripts/kconfig/qconf.cc)
Could you please consider posting this patch to linux-kbuild ML because this may be, I think, acceptable in Linux too.
On systems where both gcc and clang are installed, we can select gcc or clang by using update-alternatives (or alternative) command.
Best Regards Masahiro Yamada
Hi Jeroen, Masahiro,
On Thu, 31 Jul 2014 19:01:22 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer
For consistency,
HOSTCXX = c++
?
So, Jeroen, what is your pick ? Will you send a v3 7/8, or are you sticking with g++?
(or if everyone agrees, I could to the change to v2 7/8 when applying it, and add a comment about it in the commit message.)
Amicalement,
Hello Albert,
On 09-09-14 16:31, Albert ARIBAUD wrote:
On Thu, 31 Jul 2014 19:01:22 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer
For consistency,
HOSTCXX = c++
?
So, Jeroen, what is your pick ? Will you send a v3 7/8, or are you sticking with g++?
(or if everyone agrees, I could to the change to v2 7/8 when applying it, and add a comment about it in the commit message.)
I did check Masahiro's statement that cpp is actually used in u-boot and it is; make xconfig uses it, if I recall correctly. And as Masahiro suggested I sent it to linux-kbuild mailinglist, but I am in the impression it never arrived, perhaps I need to be subscribed or something...
Anyway, Masahiro's suggestion is sound and I am fine you changing it while applying. I tested it on FreeBSD, Xubuntu and Debian.
Regards, Jeroen
Hi Jeroen,
On Tue, 09 Sep 2014 19:34:44 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Hello Albert,
On 09-09-14 16:31, Albert ARIBAUD wrote:
On Thu, 31 Jul 2014 19:01:22 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer
For consistency,
HOSTCXX = c++
?
So, Jeroen, what is your pick ? Will you send a v3 7/8, or are you sticking with g++?
(or if everyone agrees, I could to the change to v2 7/8 when applying it, and add a comment about it in the commit message.)
I did check Masahiro's statement that cpp is actually used in u-boot and it is; make xconfig uses it, if I recall correctly. And as Masahiro suggested I sent it to linux-kbuild mailinglist, but I am in the impression it never arrived, perhaps I need to be subscribed or something...
Anyway, Masahiro's suggestion is sound and I am fine you changing it while applying. I tested it on FreeBSD, Xubuntu and Debian.
I've tried building rpi_b as per the README, but I keep getting
/usr/bin/as: unrecognized option '-mfloat-abi=soft' clang: error: assembler command failed with exit code 1 (use -v to see invocation)
Shouldn't rpi_b build properly since it is the one given as an example for Debian-based building?
Regards, Jeroen
Amicalement,
Hi,
On 09-09-14 21:59, Albert ARIBAUD wrote:
Hi Jeroen,
On Tue, 09 Sep 2014 19:34:44 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Hello Albert,
On 09-09-14 16:31, Albert ARIBAUD wrote:
On Thu, 31 Jul 2014 19:01:22 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer
For consistency,
HOSTCXX = c++
?
So, Jeroen, what is your pick ? Will you send a v3 7/8, or are you sticking with g++?
(or if everyone agrees, I could to the change to v2 7/8 when applying it, and add a comment about it in the commit message.)
I did check Masahiro's statement that cpp is actually used in u-boot and it is; make xconfig uses it, if I recall correctly. And as Masahiro suggested I sent it to linux-kbuild mailinglist, but I am in the impression it never arrived, perhaps I need to be subscribed or something...
Anyway, Masahiro's suggestion is sound and I am fine you changing it while applying. I tested it on FreeBSD, Xubuntu and Debian.
I've tried building rpi_b as per the README, but I keep getting
/usr/bin/as: unrecognized option '-mfloat-abi=soft' clang: error: assembler command failed with exit code 1 (use -v to see invocation)
Shouldn't rpi_b build properly since it is the one given as an example for Debian-based building?
As discussed with Albert there seems to be some Debian / Ubuntu specific changes to the package causing trouble. Checking out the llvm 3.5 branch works fine, using the package fails with above mentioned error.
No idea why at the moment, patches are fine...
Regards, Jeroen
Hello Albert,
On 09-09-14 21:59, Albert ARIBAUD wrote:
Hi Jeroen,
On Tue, 09 Sep 2014 19:34:44 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
I've tried building rpi_b as per the README, but I keep getting
/usr/bin/as: unrecognized option '-mfloat-abi=soft' clang: error: assembler command failed with exit code 1 (use -v to see invocation)
Shouldn't rpi_b build properly since it is the one given as an example for Debian-based building?
ok, this turns out to be a simple issue. The gas for arm is not found since CROSS_COMPILE has a trailing dash. When it is removed the correct as is picked up (and libs is found automagically as well). Both the 3.4 and 3.5 binary packages from Ubuntu are able to compile u-boot with this change, so the README should be updated.
Regards, Jeroen
This should work: make HOSTCC=clang CC="clang -target arm-linux-gnueabi -mllvm -arm-use-movt=0 -no-integrated-as" V=1 all
Hi Jeroen,
On Tue, 09 Sep 2014 19:34:44 +0200 Jeroen Hofstee jeroen@myspectrum.nl wrote:
Hello Albert,
On 09-09-14 16:31, Albert ARIBAUD wrote:
On Thu, 31 Jul 2014 19:01:22 +0900, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
HOSTCXX = g++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer
For consistency,
HOSTCXX = c++
?
So, Jeroen, what is your pick ? Will you send a v3 7/8, or are you sticking with g++?
(or if everyone agrees, I could to the change to v2 7/8 when applying it, and add a comment about it in the commit message.)
I did check Masahiro's statement that cpp is actually used in u-boot and it is; make xconfig uses it, if I recall correctly. And as Masahiro suggested I sent it to linux-kbuild mailinglist, but I am in the impression it never arrived, perhaps I need to be subscribed or something...
I received it because you cced me.
But I am afraid it did not reach the kbuild ML because I could not find it in the kbuild ML archive.
I recommend to subscribe to the ML and then resend your patch.
To subscribe to it, visit the following and click 'subscribe' http://vger.kernel.org/vger-lists.html#linux-kbuild
The kbuild ML does not have much volume. I guess you won't be annoyed.
To attract the attention of key persons, I'd like to suggest:
- add 'kbuild: ' prefix to the subject like:
kbuild: default to cc/c++ for host compiler
- you can cc linux-kernel@vger.kernel.org too. (scripts/get_maintainer.pl will do it automatically. But I am not sure if it can be reached from unsubscribers. this ML has a huge volume..)
Best Regards Masahiro Yamada
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- doc/README.clang | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 doc/README.clang
diff --git a/doc/README.clang b/doc/README.clang new file mode 100644 index 0000000..6d84f37 --- /dev/null +++ b/doc/README.clang @@ -0,0 +1,56 @@ +The biggest problem when trying to compile U-boot with clang is that +almost all archs rely on storing gd in a global register and clang user +manual states: "clang does not support global register variables; this +is unlikely to be implemented soon because it requires additional LLVM +backend support." + +Since version 3.4 the ARM backend can be instructed to leave r9 alone. +Global registers themselves are not supported so some inline assembly is +used to get its value. This does lead to larger code then strictly +necessary, but at least works. + +NOTE: target compilation only work for _some_ ARM boards at the moment. +Also Aarch64 is not supported: Most notably boards with aren't using +the generic board will fail to compile, but since these are expected +to be converted this will solve itself. Boards which reassign gd in c +will also fail to compile, but there is in no strict reason to do so +in the ARM world, since crt0.S takes care of this. These assignments +can be avoided by changing init call but this is not in mainline yet. + +NOTE: without the -mllvm -arm-use-movt=0 flags u-boot will compile +fine, but llvm might hardcode addresses in movw / movt pairs, which +cannot be relocated and u-boot will fail at runtime. + +Debian (based) +-------------- +The Debian based clang package does not have ARM support at the moment. +It must be compiled manually. + +To compile U-Boot with clang on linux without IAS use e.g.: +export CROSS_COMPILE=arm-linux-gnueabi- +make HOSTCC=clang CC="clang -target $CROSS_COMPILE -B/usr/arm-linux-gnueabi/lib/ -mllvm -arm-use-movt=0 -no-integrated-as" rpi_b_defconfig +make HOSTCC=clang CC="clang -target $CROSS_COMPILE -B/usr/arm-linux-gnueabi/lib/ -mllvm -arm-use-movt=0 -no-integrated-as" all V=1 -j8 + +FreeBSD 11 (Current): +-------------------- +Since llvm 3.4 is currently in the base system, the integrated as is +incapable of building U-Boot. Therefore gas from devel/arm-eabi-binutils +is used instead. It needs a symlinks to be picked up correctly though: + +ln -s /usr/local/bin/arm-eabi-as /usr/bin/arm-freebsd-eabi-as + +# The following commands compile U-Boot using the clang xdev toolchain. +# NOTE: CROSS_COMPILE and target differ on purpose! +export CROSS_COMPILE=arm-eabi- +gmake CC="clang -target arm-freebsd-eabi --sysroot /usr/arm-freebsd -no-integrated-as -mllvm -arm-use-movt=0" rpi_b_defconfig +gmake CC="clang -target arm-freebsd-eabi --sysroot /usr/arm-freebsd -no-integrated-as -mllvm -arm-use-movt=0" -j8 + +Given that u-boot will default to gcc, above commands can be +simplified with a simple wrapper script, listed below. + +/usr/local/bin/arm-eabi-gcc +--- +#!/bin/sh + +exec clang -target arm-freebsd-eabi --sysroot /usr/arm-freebsd -no-integrated-as -mllvm -arm-use-movt=0 "$@" +
Changes since v2: - As Albert pointed out the clang instructions don't work with Debian based binary packages. While I was aware of that it is for a different reason then I thought, it is not that ARM is not enabled as a backend but older versions are a bit more picky on the target argument and don't tolerate a trailing dash to it as used for CROSS_COMPILE etc. The README is updated accordingly.
As a side note clang3.5-svn as shipped in Ubuntu is not the 3.5 release but an snapshot of some svn commit and hence explain why the recompiled 3.5 can behave different then the ubuntu clang-3.5. Since it misses some patches, the clang3.5-svn can build less boards then 3.4 or an actual 3.5 release.
- While add it, include Masahiro suggestion to also use c++ instead of g++. - Drop dependencies from the cover-letter as they are merged. - only patch 7/8 and 8/8 are reposted. 1..6 are the same as v2.
Changes since v1 (RFC): - Update the commit message for -Werror when polling for cc-options. As pointed out by Masahiro this is not needed for all arguments, but only for warning options. - drop SOC specific patch from this patchset, I will post it seperately. - Do implement get_gd for AARCH64. - Drop the memset patch for clearing gd / not reassinging gd and use Simon his version. - Drop the patch for using gnu inline version, since targets using the generic board no longer need it. - Limit update gd in board_init_r to ARM / ARM64, since powerpc does need it. - change __inline to inline and drop volatile from get_gd to make checkpatch happy. - add patch workaround for generated constants - add patch default to cc for host compiler - update README.clang - add SOB / cc
Jeroen Hofstee (8): board_r: ARM[64] do not set gd again ARM: SPL: do not set gd again cc-option: also detect unsupported warnings options ARM: make gd a function for clang eabi_compat: add __aeabi_memcpy __aeabi_memset clang: workaround for generated constants Makefile: default to cc for host compiler README.clang: build command with clang
Kbuild | 3 +- Makefile | 4 +-- arch/arm/include/asm/global_data.h | 25 +++++++++++++++++ arch/arm/lib/eabi_compat.c | 15 ++++++++-- arch/arm/lib/spl.c | 3 -- common/board_r.c | 2 +- doc/README.clang | 56 ++++++++++++++++++++++++++++++++++++++ include/linux/kbuild.h | 6 ++-- scripts/Kbuild.include | 4 +-- 9 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 doc/README.clang
Since the host compiler might not be gcc but e.g. clang default to cc/c++ to invoke it.
cc: Masahiro Yamada yamada.m@jp.panasonic.com cc: Tom Rini trini@ti.com Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index fdda3ec..42263e4 100644 --- a/Makefile +++ b/Makefile @@ -197,8 +197,8 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \ else if [ -x /bin/bash ]; then echo /bin/bash; \ else echo sh; fi ; fi)
-HOSTCC = gcc -HOSTCXX = g++ +HOSTCC = cc +HOSTCXX = c++ HOSTCFLAGS = -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer HOSTCXXFLAGS = -O2
Cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- doc/README.clang | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 doc/README.clang
diff --git a/doc/README.clang b/doc/README.clang new file mode 100644 index 0000000..9ad689f --- /dev/null +++ b/doc/README.clang @@ -0,0 +1,56 @@ +The biggest problem when trying to compile U-boot with clang is that +almost all archs rely on storing gd in a global register and clang user +manual states: "clang does not support global register variables; this +is unlikely to be implemented soon because it requires additional LLVM +backend support." + +Since version 3.4 the ARM backend can be instructed to leave r9 alone. +Global registers themselves are not supported so some inline assembly is +used to get its value. This does lead to larger code then strictly +necessary, but at least works. + +NOTE: target compilation only work for _some_ ARM boards at the moment. +Also Aarch64 is not supported: Most notably boards which aren't using +the generic board will fail to compile, but since those are expected +to be converted this will solve itself. Boards which reassign gd in c +will also fail to compile, but there is in no strict reason to do so +in the ARM world, since crt0.S takes care of this. These assignments +can be avoided by changing the init calls but this is not in mainline yet. + +NOTE: without the -mllvm -arm-use-movt=0 flags u-boot will compile +fine, but llvm might hardcode addresses in movw / movt pairs, which +cannot be relocated and u-boot will fail at runtime. + +Debian (based) +-------------- +Binary packages can be installed as usual, e.g.: +sudo apt-get install clang + +To compile U-Boot with clang on linux without IAS use e.g.: +export TRIPLET=arm-linux-gnueabi && export CROSS_COMPILE="$TRIPLET-" +make HOSTCC=clang CC="clang -target $TRIPLET -mllvm -arm-use-movt=0 -no-integrated-as" rpi_b_defconfig +make HOSTCC=clang CC="clang -target $TRIPLET -mllvm -arm-use-movt=0 -no-integrated-as" all V=1 -j8 + +FreeBSD 11 (Current): +-------------------- +Since llvm 3.4 is currently in the base system, the integrated as is +incapable of building U-Boot. Therefore gas from devel/arm-eabi-binutils +is used instead. It needs a symlinks to be picked up correctly though: + +ln -s /usr/local/bin/arm-eabi-as /usr/bin/arm-freebsd-eabi-as + +# The following commands compile U-Boot using the clang xdev toolchain. +# NOTE: CROSS_COMPILE and target differ on purpose! +export CROSS_COMPILE=arm-eabi- +gmake CC="clang -target arm-freebsd-eabi --sysroot /usr/arm-freebsd -no-integrated-as -mllvm -arm-use-movt=0" rpi_b_defconfig +gmake CC="clang -target arm-freebsd-eabi --sysroot /usr/arm-freebsd -no-integrated-as -mllvm -arm-use-movt=0" -j8 + +Given that u-boot will default to gcc, above commands can be +simplified with a simple wrapper script, listed below. + +/usr/local/bin/arm-eabi-gcc +--- +#!/bin/sh + +exec clang -target arm-freebsd-eabi --sysroot /usr/arm-freebsd -no-integrated-as -mllvm -arm-use-movt=0 "$@" +
Hi Jeroen,
On Wed, 10 Sep 2014 20:08:50 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Changes since v2:
As Albert pointed out the clang instructions don't work with Debian based binary packages. While I was aware of that it is for a different reason then I thought, it is not that ARM is not enabled as a backend but older versions are a bit more picky on the target argument and don't tolerate a trailing dash to it as used for CROSS_COMPILE etc. The README is updated accordingly.
As a side note clang3.5-svn as shipped in Ubuntu is not the 3.5 release but an snapshot of some svn commit and hence explain why the recompiled 3.5 can behave different then the ubuntu clang-3.5. Since it misses some patches, the clang3.5-svn can build less boards then 3.4 or an actual 3.5 release.
While add it, include Masahiro suggestion to also use c++ instead of g++.
Drop dependencies from the cover-letter as they are merged.
only patch 7/8 and 8/8 are reposted. 1..6 are the same as v2.
Thanks, tested building rpi_b, it works now.
The, tested on versatileqemu out of curiosity and got the following results:
1.
clang warns about Unused static functions in common/console.c, namely console_printdevs and console_doenv (1). Why gcc does not flag this? We have -Wall set which is supposed to imply -Wunused-functions.
There is also an unused variable warning in disk/part.c28 (block_drvr). I haven't looked at why clang warns about it and not gcc, but it could raise the same question as the functions above.
2.
clang errors on arch/arm/lib/cache.c:28 for this: asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); and that is a clang mistake, as for ARM926EJS r15 is a valid (albeit quite special semantically) Rd for Test and Clean DCache, see page 2-24.
Jeroen, do you feel like raising point 2 to the clang/LLVM folks?
(1) and BTW it's not like the functions are used in some configuration other than versatileqemu; they're completely unused.
Other than that, the patch series seems to be good. I'll apply it soonish.
Amicalement,
Hello Albert,
On do, 2014-09-11 at 10:32 +0200, Albert ARIBAUD wrote:
On Wed, 10 Sep 2014 20:08:50 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Changes since v2:
As Albert pointed out the clang instructions don't work with Debian based binary packages. While I was aware of that it is for a different reason then I thought, it is not that ARM is not enabled as a backend but older versions are a bit more picky on the target argument and don't tolerate a trailing dash to it as used for CROSS_COMPILE etc. The README is updated accordingly.
As a side note clang3.5-svn as shipped in Ubuntu is not the 3.5 release but an snapshot of some svn commit and hence explain why the recompiled 3.5 can behave different then the ubuntu clang-3.5. Since it misses some patches, the clang3.5-svn can build less boards then 3.4 or an actual 3.5 release.
While add it, include Masahiro suggestion to also use c++ instead of g++.
Drop dependencies from the cover-letter as they are merged.
only patch 7/8 and 8/8 are reposted. 1..6 are the same as v2.
Thanks, tested building rpi_b, it works now.
The, tested on versatileqemu out of curiosity and got the following results:
clang warns about Unused static functions in common/console.c, namely console_printdevs and console_doenv (1). Why gcc does not flag this? We have -Wall set which is supposed to imply -Wunused-functions.
It is a gcc feature, see [1]: "Warn whenever a static function is declared but not defined or a _non-inline static function_ is unused. This warning is enabled by -Wall."
There is also an unused variable warning in disk/part.c28 (block_drvr). I haven't looked at why clang warns about it and not gcc, but it could raise the same question as the functions above.
Also a gcc feature, see [2]. The constant is indeed not used.
clang errors on arch/arm/lib/cache.c:28 for this: asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); and that is a clang mistake, as for ARM926EJS r15 is a valid (albeit quite special semantically) Rd for Test and Clean DCache, see page 2-24.
This is the integrated-as complaining (the README tells you to disable it for the moment). The clang folks push UAL hard, up to a point we need to think about minimum gcc version etc. To avoid that, I just left out such changes and just use gas instead, at least for the time being. Below are some changes to compile versatileqemu with llvm integrated-as and gcc/gas. No idea if it actually boots though.
Jeroen, do you feel like raising point 2 to the clang/LLVM folks?
It is removed on purpose as far as I understood. I don't think they regards it as a bug, see obfuscated patch below.
Other than that, the patch series seems to be good. I'll apply it soonish.
Thanks, Jeroen
[1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13029
s/~/-/g
~~~
From dbc16575725693378ad5dac84d6fb945545c3b63 Mon Sep 17 00:00:00 2001
From: Jeroen Hofstee jeroen@myspectrum.nl Date: Thu, 11 Sep 2014 12:46:35 +0200 Subject: [PATCH] versatileqemu ual build
~~~ arch/arm/cpu/arm926ejs/cache.c | 2 +~ arch/arm/cpu/arm926ejs/start.S | 2 +~ arch/arm/lib/cache.c | 2 +~ include/configs/versatile.h | 2 ++ 4 files changed, 5 insertions(+), 3 deletions(~)
diff ~~git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c index e86c2ed..b1aae13 100644 ~~~ a/arch/arm/cpu/arm926ejs/cache.c +++ b/arch/arm/cpu/arm926ejs/cache.c @@ ~22,7 +22,7 @@ void flush_dcache_all(void) { asm volatile( "0:" ~ "mrc p15, 0, r15, c7, c14, 3\n" + "mrc p15, 0, apsr_nzcv, c7, c14, 3\n" "bne 0b\n" "mcr p15, 0, %0, c7, c10, 4\n" : : "r"(0) : "memory" diff ~~git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 8eb2494..b9d2ae2 100644 ~~~ a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ ~78,7 +78,7 @@ cpu_init_crit: */ mov r0, #0 flush_dcache: ~ mrc p15, 0, r15, c7, c10, 3 + mrc p15, 0, apsr_nzcv, c7, c10, 3 bne flush_dcache
mcr p15, 0, r0, c8, c7, 0 /* invalidate TLB */ diff ~~git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c index 4e597a4..d12ea2b 100644 ~~~ a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ ~25,7 +25,7 @@ __weak void flush_cache(unsigned long start, unsigned long size)
#ifdef CONFIG_ARM926EJS /* test and clean, page 2~23 of arm926ejs manual */ ~ asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); + asm("0: mrc p15, 0, apsr_nzcv, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); /* disable write buffer as well (page 2~22) */ asm("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); #endif /* CONFIG_ARM926EJS */ diff ~~git a/include/configs/versatile.h b/include/configs/versatile.h index 29c32fe..52d2af3 100644 ~~~ a/include/configs/versatile.h +++ b/include/configs/versatile.h @@ ~23,6 +23,8 @@ #define CONFIG_VERSATILE 1 /* in Versatile Platform Board */ #define CONFIG_ARCH_VERSATILE 1 /* Specifically, a Versatile */
+#define CONFIG_SYS_GENERIC_BOARD + #define CONFIG_SYS_MEMTEST_START 0x100000 #define CONFIG_SYS_MEMTEST_END 0x10000000
~~ 2.1.0
Hi Jeroen,
On Thu, 11 Sep 2014 13:17:20 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Hello Albert,
On do, 2014-09-11 at 10:32 +0200, Albert ARIBAUD wrote:
On Wed, 10 Sep 2014 20:08:50 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Changes since v2:
As Albert pointed out the clang instructions don't work with Debian based binary packages. While I was aware of that it is for a different reason then I thought, it is not that ARM is not enabled as a backend but older versions are a bit more picky on the target argument and don't tolerate a trailing dash to it as used for CROSS_COMPILE etc. The README is updated accordingly.
As a side note clang3.5-svn as shipped in Ubuntu is not the 3.5 release but an snapshot of some svn commit and hence explain why the recompiled 3.5 can behave different then the ubuntu clang-3.5. Since it misses some patches, the clang3.5-svn can build less boards then 3.4 or an actual 3.5 release.
While add it, include Masahiro suggestion to also use c++ instead of g++.
Drop dependencies from the cover-letter as they are merged.
only patch 7/8 and 8/8 are reposted. 1..6 are the same as v2.
Thanks, tested building rpi_b, it works now.
The, tested on versatileqemu out of curiosity and got the following results:
clang warns about Unused static functions in common/console.c, namely console_printdevs and console_doenv (1). Why gcc does not flag this? We have -Wall set which is supposed to imply -Wunused-functions.
It is a gcc feature, see [1]: "Warn whenever a static function is declared but not defined or a _non-inline static function_ is unused. This warning is enabled by -Wall."
Ok, I'll assume there is some logic in there, but then, clang does not follow that logic -- so which one is the 'good' one? Or maybe that's the same as the second issue, where...
There is also an unused variable warning in disk/part.c28 (block_drvr). I haven't looked at why clang warns about it and not gcc, but it could raise the same question as the functions above.
Also a gcc feature, see [2]. The constant is indeed not used.
... apparently, it's a case of trying to balance false positives and missed true issue, and each team has its own view of where to set the limit. :/
(anyway -- here clearly, clang is right in warning us and gcc is wrong in not doing it)
clang errors on arch/arm/lib/cache.c:28 for this: asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); and that is a clang mistake, as for ARM926EJS r15 is a valid (albeit quite special semantically) Rd for Test and Clean DCache, see page 2-24.
This is the integrated-as complaining (the README tells you to disable it for the moment). The clang folks push UAL hard, up to a point we need to think about minimum gcc version etc. To avoid that, I just left out such changes and just use gas instead, at least for the time being. Below are some changes to compile versatileqemu with llvm integrated-as and gcc/gas. No idea if it actually boots though.
Jeroen, do you feel like raising point 2 to the clang/LLVM folks?
It is removed on purpose as far as I understood. I don't think they regards it as a bug, see obfuscated patch below.
Other than that, the patch series seems to be good. I'll apply it soonish.
Thanks, Jeroen
~ "mrc p15, 0, r15, c7, c14, 3\n"
"mrc p15, 0, apsr_nzcv, c7, c14, 3\n"
Is this is a hack to set the Rd field of the mrc instruction to a value equal to what "r15" would have given, but fooling clang by using an unrelated and, in this context, meaningless, symbol instead of "r15"?
Amicalement,
Hello Albert,
On 11-09-14 15:31, Albert ARIBAUD wrote:
Thanks, tested building rpi_b, it works now.
The, tested on versatileqemu out of curiosity and got the following results:
clang warns about Unused static functions in common/console.c, namely console_printdevs and console_doenv (1). Why gcc does not flag this? We have -Wall set which is supposed to imply -Wunused-functions. It is a gcc feature, see [1]: "Warn whenever a static function is declared but not defined or a _non-inline static function_ is unused. This warning is enabled by -Wall."
Ok, I'll assume there is some logic in there, but then, clang does not follow that logic -- so which one is the 'good' one? Or maybe that's the same as the second issue, where...
well I don't know the details, but the compiler should not emit a warning if the static inline came from a header file, perhaps that is motivation behind it. Anyway I have a branch with 60 patches or so fixing warnings, don't bother too much about them for the time being.
clang errors on arch/arm/lib/cache.c:28 for this: asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); and that is a clang mistake, as for ARM926EJS r15 is a valid (albeit quite special semantically) Rd for Test and Clean DCache, see page 2-24.
This is the integrated-as complaining (the README tells you to disable it for the moment). The clang folks push UAL hard, up to a point we need to think about minimum gcc version etc. To avoid that, I just left out such changes and just use gas instead, at least for the time being. Below are some changes to compile versatileqemu with llvm integrated-as and gcc/gas. No idea if it actually boots though.
[..]
~ "mrc p15, 0, r15, c7, c14, 3\n"
"mrc p15, 0, apsr_nzcv, c7, c14, 3\n"Is this is a hack to set the Rd field of the mrc instruction to a value equal to what "r15" would have given, but fooling clang by using an unrelated and, in this context, meaningless, symbol instead of "r15"?
Nope, it is UAL syntax, binutils agrees:
arm-linux-gnueabi-objdump -S u-boot
flush_dcache: mrc p15, 0, r15, c7, c10, 3 10320: ee17ff7a mrc 15, 0, APSR_nzcv, cr7, cr10, {3}
Clang is just pushing it a bit harder. I have a branch for that too, but as said, I will let it bit-rot for a while, since that would require about minimal gcc versions and other boring stuff.
Regards, Jeroen
Hi Jeroen,
Correction on the asm stuff:
On Thu, 11 Sep 2014 13:17:20 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
clang errors on arch/arm/lib/cache.c:28 for this: asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); and that is a clang mistake, as for ARM926EJS r15 is a valid (albeit quite special semantically) Rd for Test and Clean DCache, see page 2-24.
This is the integrated-as complaining (the README tells you to disable it for the moment). The clang folks push UAL hard, up to a point we need to think about minimum gcc version etc. To avoid that, I just left out such changes and just use gas instead, at least for the time being. Below are some changes to compile versatileqemu with llvm integrated-as and gcc/gas. No idea if it actually boots though.
Actually, I had the -no-integrated-as then and have just re-tested now, making sure I have it and get the error above. For some reason, despite the -no-integrated-as option, the internal assembler is invoked.
Amicalement,
Hello Albert,
On 11-09-14 17:43, Albert ARIBAUD wrote:
Hi Jeroen,
Correction on the asm stuff:
On Thu, 11 Sep 2014 13:17:20 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
clang errors on arch/arm/lib/cache.c:28 for this: asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory"); and that is a clang mistake, as for ARM926EJS r15 is a valid (albeit quite special semantically) Rd for Test and Clean DCache, see page 2-24.
This is the integrated-as complaining (the README tells you to disable it for the moment). The clang folks push UAL hard, up to a point we need to think about minimum gcc version etc. To avoid that, I just left out such changes and just use gas instead, at least for the time being. Below are some changes to compile versatileqemu with llvm integrated-as and gcc/gas. No idea if it actually boots though.
Actually, I had the -no-integrated-as then and have just re-tested now, making sure I have it and get the error above. For some reason, despite the -no-integrated-as option, the internal assembler is invoked.
You don't happen to be testing with the clang 3.5 minus a half / non release (svn 201651) right? As I mentioned before, it will do you more harm then good. I cannot reproduce this with an 3.4 nor 3.5 release.
Regards, Jeroen
Hi Jeroen,
On Wed, 10 Sep 2014 20:08:50 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Changes since v2:
As Albert pointed out the clang instructions don't work with Debian based binary packages. While I was aware of that it is for a different reason then I thought, it is not that ARM is not enabled as a backend but older versions are a bit more picky on the target argument and don't tolerate a trailing dash to it as used for CROSS_COMPILE etc. The README is updated accordingly.
As a side note clang3.5-svn as shipped in Ubuntu is not the 3.5 release but an snapshot of some svn commit and hence explain why the recompiled 3.5 can behave different then the ubuntu clang-3.5. Since it misses some patches, the clang3.5-svn can build less boards then 3.4 or an actual 3.5 release.
While add it, include Masahiro suggestion to also use c++ instead of g++.
Drop dependencies from the cover-letter as they are merged.
only patch 7/8 and 8/8 are reposted. 1..6 are the same as v2.
Series (patches 1-6 of v2, 7 and 8 of v3) applied to u-boot-arm/master, thanks!
(still 2 warnings and 5 errors at the top of u-boot-arm/master, but this series does not make things any worse)
Amicalement,
participants (8)
-
Albert ARIBAUD -
Jeroen Hofstee -
Jeroen Hofstee -
Masahiro Yamada -
Simon Glass -
Stefan Roese -
Tim Harvey -
York Sun