Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
Hi Jeroen, On Wed, 10 Sep 2014 20:08:50 +0200, Jeroen Hofstee jer...@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, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
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
jer...@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.
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.
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.
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 jer...@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
Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
Hi Jeroen, On Thu, 11 Sep 2014 13:17:20 +0200, Jeroen Hofstee jer...@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 jer...@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. 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) 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. 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, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
Hi Jeroen, On Wed, 10 Sep 2014 20:08:50 +0200, Jeroen Hofstee jer...@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, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
Hi Jeroen, Correction on the asm stuff: On Thu, 11 Sep 2014 13:17:20 +0200, Jeroen Hofstee jer...@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, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
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 jer...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
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:
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.
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.
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.
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: ee17ff7amrc 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 0/8] add clang support for some ARM boards
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 -- 2.1.0 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
