Re: aarch64 gcc kernel compilation
On Jul 17, 10:39am, n...@gmx.com (Kamil Rytarowski) wrote: -- Subject: Re: aarch64 gcc kernel compilation | I was thinking about this one: | | http://netbsd.org/~kamil/patch-00067-conditional-u128.txt | | It should do the trick for everybody - including rumpkernel users - and | support aarch64 with disabled 128bit types (I worked on them on Linux). I am fine with that one. christos
Re: aarch64 gcc kernel compilation
On 17.07.2018 07:23, Ryo Shimizu wrote: > > Can we drop it? The __uint128_t type is not used anywhere else in > aarch64 subdirs. > > fortunately, we don't use member of fpreg, use just only as container. > Is this patch enough? > I was thinking about this one: http://netbsd.org/~kamil/patch-00067-conditional-u128.txt It should do the trick for everybody - including rumpkernel users - and support aarch64 with disabled 128bit types (I worked on them on Linux). signature.asc Description: OpenPGP digital signature
Re: aarch64 gcc kernel compilation
On Tue, Jul 17, 2018 at 02:23:36PM +0900, Ryo Shimizu wrote:
> union fpelem {
> uint64_t u64[2];
> - __uint128_t u128[1] __aligned(16);
> + /* __uint128_t u128[1] __aligned(16); */
> };
I like the alignement parts of the change, but I seriously do not
understand why the __uint128_t is a bad thing.
I'd be fine if that line is #if'd out for compilers not supporting
it, if they exist.
No support in printf and friends for this type is a strange justification
for removing a very natural representation for kernel purposes here
if it does not actually hurt (and we never printf it this way
at all, and if needed just a hexdump of the fpelem would be good enough).
I am not objecting this patch in general, but so far no coherent
proposal has been made why this should happen.
Martin
Re: aarch64 gcc kernel compilation
Can we drop it? The __uint128_t type is not used anywhere else in
aarch64 subdirs.
fortunately, we don't use member of fpreg, use just only as container.
Is this patch enough?
cvs -q diff -aup pcb.h reg.h
Index: pcb.h
===
RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/aarch64/include/pcb.h,v
retrieving revision 1.1
diff -a -u -p -r1.1 pcb.h
--- pcb.h 10 Aug 2014 05:47:38 - 1.1
+++ pcb.h 17 Jul 2018 04:59:37 -
@@ -45,6 +45,10 @@ struct md_coredump {
struct fpreg fpreg;
};
+/* fpreg should be aligned 16 */
+CTASSERT((offsetof(struct pcb, pcb_fpregs) & 15) == 0);
+CTASSERT((offsetof(struct md_coredump, fpreg) & 15) == 0);
+
#elif defined(__arm__)
#include
Index: reg.h
===
RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/aarch64/include/reg.h,v
retrieving revision 1.2
diff -a -u -p -r1.2 reg.h
--- reg.h 1 Apr 2018 04:35:03 - 1.2
+++ reg.h 17 Jul 2018 04:58:07 -
@@ -44,14 +44,14 @@ struct reg {
union fpelem {
uint64_t u64[2];
- __uint128_t u128[1] __aligned(16);
+ /* __uint128_t u128[1] __aligned(16); */
};
struct fpreg {
union fpelem fp_reg[32];
uint32_t fpcr;
uint32_t fpsr;
-};
+} __aligned(16);
#elif defined(__arm__)
--
ryo shimizu
Re: aarch64 gcc kernel compilation
On 16.07.2018 11:07, Kamil Rytarowski wrote: > On 16.07.2018 10:50, Kamil Rytarowski wrote: >> On 16.07.2018 00:00, Kamil Rytarowski wrote: >>> On 15.07.2018 20:08, Christos Zoulas wrote: Hi, Gcc is now working on aarch64 but the kernel does not compile because of some idiomatic clang code that is not supported by gcc (at least gcc-6) To define constants, it uses: static const uintmax_t FOO = __BIT(9), BAR = FOO; While this is nice, specially for the debugger, it produces an error in gcc. While fixing these is easy, gcc also complains about using the constants as switch labels. Thus it is better to just nukem all and rewrite them as: #define FOO __BIT(9) #define BAR FOO Should I go ahead and do it, or there is a smarter solution? christos >>> >>> I used to have problems to build rumpkernel aarch64 on Linux with GCC >>> (some years ago) due to usage __uint128_t in reg.h. >>> >>> Can we drop it? The __uint128_t type is not used anywhere else in >>> aarch64 subdirs. >>> >>> It's used in assembly in FPREG_Q0-FPREQ_Q31 in cpuswitch.S. The same >>> optimization can be done without the usage of __uint128_t, probably just >>> need for proper alignment of fp_reg (15). >> >> 16* >> >>> >>> There is also some mysterious fallout that General Purpose Registers in >>> core files are shipped with 128bit containers. It's not compatible with >>> LLDB and requires needless generic work for no purpose. >>> >>> I can try to prepare a patch blindly and share with aarch64 owners. >>> >> >> Looking deeper, there are various reports regarding aarch64 128-bit >> broken support. >> >> "Be careful of GCC's __uint128_t. It caused us problems on a number of >> platforms, like ARM64, ARMEL and S/390. We had to give up using it >> because it was so buggy. For example, GCC calculated the result of u = >> 93 - 0 - 0 - 0 (using the 128-bit types) as 18446744073709551615 on ARM64" >> >> https://stackoverflow.com/questions/11656241/how-to-print-uint128-t-number-using-gcc >> >> There are no utility features for such numbers such as PRIu128, no >> support in printf(3), snprintf(3) etc. >> >> I will prepare a patch for removal of this from public machine headers. >> > > I was asked to provide some links to gcc bugzilla: > > https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__int128_t > https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__uint128_t > > My reason is unportable construct of reg.h, no utility functions for > 128bits and alien style core files with 128bit containers for registers. > This has been rejected by Martin. signature.asc Description: OpenPGP digital signature
Re: aarch64 gcc kernel compilation
>Gcc is now working on aarch64 but the kernel does not compile because of
>some idiomatic clang code that is not supported by gcc (at least gcc-6)
>
>To define constants, it uses:
>
>static const uintmax_t
>FOO = __BIT(9),
>BAR = FOO;
>
>While this is nice, specially for the debugger, it produces an error
>in gcc. While fixing these is easy, gcc also complains about using the
>constants as switch labels. Thus it is better to just nukem all and
>rewrite them as:
>
>#define FOO __BIT(9)
>#define BAR FOO
>
>Should I go ahead and do it, or there is a smarter solution?
Yes, please!
I tested with below script in my environment (clang), there seems to be no
problem.
perl -i.old a64armreg_conv.pl aarch64/include/armreg.h
# a64armreg_conv.pl
while (<>) {
if (m#^static\s+(const\s*)?uintmax_t\s*(//.*)?$#) {
} elsif (m#^\s*(\w+)\s*=\s*(.*?)[,;]\s*//(.*)$#) {
print "#define $1 $2 //$3\n";
} elsif (m#^\s*(\w+)\s*=\s*(.*?)[,;]\s*$#) {
print "#define $1 $2\n";
} else {
print;
}
}
--
ryo shimizu
Re: aarch64 gcc kernel compilation
On 16.07.2018 10:50, Kamil Rytarowski wrote: > On 16.07.2018 00:00, Kamil Rytarowski wrote: >> On 15.07.2018 20:08, Christos Zoulas wrote: >>> Hi, >>> >>> Gcc is now working on aarch64 but the kernel does not compile because of >>> some idiomatic clang code that is not supported by gcc (at least gcc-6) >>> >>> To define constants, it uses: >>> >>> static const uintmax_t >>> FOO = __BIT(9), >>> BAR = FOO; >>> >>> While this is nice, specially for the debugger, it produces an error >>> in gcc. While fixing these is easy, gcc also complains about using the >>> constants as switch labels. Thus it is better to just nukem all and >>> rewrite them as: >>> >>> #define FOO __BIT(9) >>> #define BAR FOO >>> >>> Should I go ahead and do it, or there is a smarter solution? >>> >>> christos >>> >> >> I used to have problems to build rumpkernel aarch64 on Linux with GCC >> (some years ago) due to usage __uint128_t in reg.h. >> >> Can we drop it? The __uint128_t type is not used anywhere else in >> aarch64 subdirs. >> >> It's used in assembly in FPREG_Q0-FPREQ_Q31 in cpuswitch.S. The same >> optimization can be done without the usage of __uint128_t, probably just >> need for proper alignment of fp_reg (15). > > 16* > >> >> There is also some mysterious fallout that General Purpose Registers in >> core files are shipped with 128bit containers. It's not compatible with >> LLDB and requires needless generic work for no purpose. >> >> I can try to prepare a patch blindly and share with aarch64 owners. >> > > Looking deeper, there are various reports regarding aarch64 128-bit > broken support. > > "Be careful of GCC's __uint128_t. It caused us problems on a number of > platforms, like ARM64, ARMEL and S/390. We had to give up using it > because it was so buggy. For example, GCC calculated the result of u = > 93 - 0 - 0 - 0 (using the 128-bit types) as 18446744073709551615 on ARM64" > > https://stackoverflow.com/questions/11656241/how-to-print-uint128-t-number-using-gcc > > There are no utility features for such numbers such as PRIu128, no > support in printf(3), snprintf(3) etc. > > I will prepare a patch for removal of this from public machine headers. > I was asked to provide some links to gcc bugzilla: https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__int128_t https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__uint128_t My reason is unportable construct of reg.h, no utility functions for 128bits and alien style core files with 128bit containers for registers. signature.asc Description: OpenPGP digital signature
Re: aarch64 gcc kernel compilation
On 15.07.2018 20:08, Christos Zoulas wrote: > Hi, > > Gcc is now working on aarch64 but the kernel does not compile because of > some idiomatic clang code that is not supported by gcc (at least gcc-6) > > To define constants, it uses: > > static const uintmax_t > FOO = __BIT(9), > BAR = FOO; > > While this is nice, specially for the debugger, it produces an error > in gcc. While fixing these is easy, gcc also complains about using the > constants as switch labels. Thus it is better to just nukem all and > rewrite them as: > > #define FOO __BIT(9) > #define BAR FOO > > Should I go ahead and do it, or there is a smarter solution? > > christos > I used to have problems to build rumpkernel aarch64 on Linux with GCC (some years ago) due to usage __uint128_t in reg.h. Can we drop it? The __uint128_t type is not used anywhere else in aarch64 subdirs. It's used in assembly in FPREG_Q0-FPREQ_Q31 in cpuswitch.S. The same optimization can be done without the usage of __uint128_t, probably just need for proper alignment of fp_reg (15). There is also some mysterious fallout that General Purpose Registers in core files are shipped with 128bit containers. It's not compatible with LLDB and requires needless generic work for no purpose. I can try to prepare a patch blindly and share with aarch64 owners. signature.asc Description: OpenPGP digital signature
aarch64 gcc kernel compilation
Hi, Gcc is now working on aarch64 but the kernel does not compile because of some idiomatic clang code that is not supported by gcc (at least gcc-6) To define constants, it uses: static const uintmax_t FOO = __BIT(9), BAR = FOO; While this is nice, specially for the debugger, it produces an error in gcc. While fixing these is easy, gcc also complains about using the constants as switch labels. Thus it is better to just nukem all and rewrite them as: #define FOO __BIT(9) #define BAR FOO Should I go ahead and do it, or there is a smarter solution? christos
