| Message ID | 20230209004823.3698203-1-giulio.benetti@benettiengineering.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arch: move global and rename variable BR2_PACKAGE_Z3_ARCH_SUPPORTS | expand |
Pardon, this had to be a RFC, I've forgotten to add it. Some architecture still lacks some float rounding but is listed. Let me know if it's possible to apply this patch and catch the other architecture failures on Autobuilder rather than check all the architectures' FPU implementation by hand(I can do it anyway if requested). Best regards
On Thu, 9 Feb 2023 01:48:23 +0100 Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > Package quickjs fails to build on architectures that don't support full > float rounding. To support all kind of float rounding FE_DOWNWARD, > FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. > Since package z3 locally implements this check let's make it global, > rename it to be generic and use it for both package quickjs and z3. > > Fixes: > http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ > http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ > http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ > http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ > > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> Apparently https://patchwork.ozlabs.org/project/buildroot/patch/20221216221934.465323-1-ju.o@free.fr/ would also need the same thing. Thomas
On 09/02/23 08:54, Thomas Petazzoni via buildroot wrote: > Subject: > Re: [Buildroot] [PATCH] arch: move global and rename variable > BR2_PACKAGE_Z3_ARCH_SUPPORTS > From: > Thomas Petazzoni via buildroot <buildroot@buildroot.org> > Date: > 09/02/23, 08:54 > > To: > Giulio Benetti <giulio.benetti@benettiengineering.com> > CC: > Julien Olivain <ju.o@free.fr>, buildroot@buildroot.org > > > On Thu, 9 Feb 2023 01:48:23 +0100 > Giulio Benetti<giulio.benetti@benettiengineering.com> wrote: > >> Package quickjs fails to build on architectures that don't support full >> float rounding. To support all kind of float rounding FE_DOWNWARD, >> FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. >> Since package z3 locally implements this check let's make it global, >> rename it to be generic and use it for both package quickjs and z3. >> >> Fixes: >> http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ >> http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ >> http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ >> http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ >> >> Signed-off-by: Giulio Benetti<giulio.benetti@benettiengineering.com> > Apparently > https://patchwork.ozlabs.org/project/buildroot/patch/20221216221934.465323-1-ju.o@free.fr/ > would also need the same thing. Yes, it requires it too once it is applied. I can modify my patch if you apply the swipl one before. No problem. Best regards
Hi Giulio, On 09/02/2023 01:48, Giulio Benetti wrote: > Package quickjs fails to build on architectures that don't support full > float rounding. To support all kind of float rounding FE_DOWNWARD, > FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. > Since package z3 locally implements this check let's make it global, > rename it to be generic and use it for both package quickjs and z3. > > Fixes: > http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ > http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ > http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ > http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ > > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> To comment a bit on that, the complete condition is a bit more tricky: - the CPU arch FPU HW must have all 4 rounding modes, - the libc must expose a fenv.h header, - the libc must also expose all 4 rouding modes. glibc generally exposes all the HW supported rounding modes. I initially excluded uclibc-ng, because in its Buildroot default configuration, fenv.h is not enabled [1]. I believe it's possible to have some uclibc-ng configuration that are enable and exposing those rounding modes. See [2]. So this proposal is correct but possibly discard the case in which libc is uclibc-ng and its (custom) configuration exposes all FE rounding modes. I am not sure if it's really an issue, as packages with such requirements are mostly tested with glibc anyway... Best regards, Julien. [1] https://git.busybox.net/buildroot/tree/package/uclibc/uClibc-ng.config [2] https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/extra/Configs/Config.in.arch?h=v1.0.42#n180
Hi Julien, On 09/02/23 20:36, Julien Olivain wrote: > > Hi Giulio, > > On 09/02/2023 01:48, Giulio Benetti wrote: >> Package quickjs fails to build on architectures that don't support full >> float rounding. To support all kind of float rounding FE_DOWNWARD, >> FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. >> Since package z3 locally implements this check let's make it global, >> rename it to be generic and use it for both package quickjs and z3. >> >> Fixes: >> http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ >> http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ >> http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ >> http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ >> >> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > > To comment a bit on that, the complete condition is a bit more tricky: > - the CPU arch FPU HW must have all 4 rounding modes, > - the libc must expose a fenv.h header, > - the libc must also expose all 4 rouding modes. thank you for the more precise explanation, everything is more clear now > glibc generally exposes all the HW supported rounding modes. I > initially excluded uclibc-ng, because in its Buildroot default > configuration, fenv.h is not enabled [1]. I believe it's possible to > have some uclibc-ng configuration that are enable and exposing those > rounding modes. See [2]. > > So this proposal is correct but possibly discard the case in which libc is > uclibc-ng and its (custom) configuration exposes all FE rounding modes. Maybe we can enable UCLIBC_HAS_FENV on uclibc-ng by default, so all architectures that UCLIBC_HAS_FLOAT they have it. Otherwise that UCLIBC_HAS_FENV gets discarded if I'm correct. > I am not sure if it's really an issue, as packages with such > requirements are mostly tested with glibc anyway... Same here for the architecture. So we'd need a valid condition that is true for the 3 points you've listed above exactly in the order you've listed them if I'm correct. Maybe we can do an intermediate step with this patch and later deal with this matrix validity check Best regards
Hello Giulio, Le 09/02/2023 à 01:48, Giulio Benetti a écrit : > Package quickjs fails to build on architectures that don't support full > float rounding. To support all kind of float rounding FE_DOWNWARD, > FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. > Since package z3 locally implements this check let's make it global, > rename it to be generic and use it for both package quickjs and z3. > > Fixes: > http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ > http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ > http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ > http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ > > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > --- > arch/Config.in | 24 ++++++++++++++++++++++++ > package/quickjs/Config.in | 4 ++-- > package/z3/Config.in | 25 +------------------------ > 3 files changed, 27 insertions(+), 26 deletions(-) > > diff --git a/arch/Config.in b/arch/Config.in > index 1c0c400a98..90846861ca 100644 > --- a/arch/Config.in > +++ b/arch/Config.in > @@ -253,6 +253,30 @@ config BR2_xtensa > > endchoice > > +# Not all architectures support float rounding. This can be found while > +# checking if fenv.h provides all four macros: > +# FE_DOWNWARD, FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD > +# See for example in glibc https://sourceware.org/git/glibc.git > +# git grep -E '^[[:space:]]*#[[:space:]]*define[[:space:]]+FE_(TONEAREST|UPWARD|DOWNWARD|TOWARDZERO)' sysdeps/ > +config BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING > + bool > + default y if BR2_aarch64 || BR2_aarch64_be > + default y if BR2_arceb || BR2_arcle > + default y if BR2_arm || BR2_armeb > + default y if BR2_i386 > + default y if BR2_m68k > + # BR2_microblaze has only FE_TONEAREST > + default y if BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el > + # BR2_nios2 has only FE_TONEAREST > + default y if BR2_or1k > + default y if BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le > + default y if BR2_riscv > + default y if BR2_s390x > + # BR2_sh has only FE_{TONEAREST,TOWARDZERO} > + default y if BR2_sparc || BR2_sparc64 > + default y if BR2_x86_64 > + # BR2_xtensa supports only uclibc which does not have fenv.h > + > # For some architectures or specific cores, our internal toolchain > # backend is not suitable (like, missing support in upstream gcc, or > # no ChipCo fork exists...) > diff --git a/package/quickjs/Config.in b/package/quickjs/Config.in > index dc466241ba..6f24787333 100644 > --- a/package/quickjs/Config.in > +++ b/package/quickjs/Config.in > @@ -1,6 +1,6 @@ > config BR2_PACKAGE_QUICKJS > bool "quickjs" > - depends on !BR2_nios2 # fenv.h lacks FE_{DOWN,UP}WARD on nios2 > + depends on BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING # fenv.h lacks FE_{DOWN,UP}WARD I would suggest to split into two separate patches: - Move and rename BR2_PACKAGE_Z3_ARCH_SUPPORTS -> BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING - Fix quickjs dependencies with BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING While at it, you can improve the commit log with Julien's comments: http://lists.busybox.net/pipermail/buildroot/2023-February/661768.html Best regards, Romain > depends on !BR2_STATIC_LIBS > # No way to check for fenv support. > depends on !BR2_TOOLCHAIN_USES_UCLIBC > @@ -15,7 +15,7 @@ config BR2_PACKAGE_QUICKJS > https://bellard.org/quickjs/ > > comment "quickjs needs a glibc or musl toolchain w/ gcc >= 4.9, host gcc >= 4.9, dynamic library" > - depends on !BR2_nios2 > + depends on BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING > depends on BR2_USE_MMU > depends on BR2_STATIC_LIBS || BR2_TOOLCHAIN_USES_UCLIBC || \ > !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || !BR2_HOST_GCC_AT_LEAST_4_9 > diff --git a/package/z3/Config.in b/package/z3/Config.in > index 8cd3128687..5ceca5dbe2 100644 > --- a/package/z3/Config.in > +++ b/package/z3/Config.in > @@ -1,33 +1,10 @@ > -# z3 supports arch for which libc fenv.h provides all four macros: > -# FE_DOWNWARD, FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD > -# See for example in glibc https://sourceware.org/git/glibc.git > -# git grep -E '^[[:space:]]*#[[:space:]]*define[[:space:]]+FE_(TONEAREST|UPWARD|DOWNWARD|TOWARDZERO)' sysdeps/ > -config BR2_PACKAGE_Z3_ARCH_SUPPORTS > - bool > - default y if BR2_aarch64 || BR2_aarch64_be > - default y if BR2_arceb || BR2_arcle > - default y if BR2_arm || BR2_armeb > - default y if BR2_i386 > - default y if BR2_m68k > - # BR2_microblaze has only FE_TONEAREST > - default y if BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el > - # BR2_nios2 has only FE_TONEAREST > - default y if BR2_or1k > - default y if BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le > - default y if BR2_riscv > - default y if BR2_s390x > - # BR2_sh has only FE_{TONEAREST,TOWARDZERO} > - default y if BR2_sparc || BR2_sparc64 > - default y if BR2_x86_64 > - # BR2_xtensa supports only uclibc which does not have fenv.h > - > config BR2_PACKAGE_Z3 > bool "z3" > depends on BR2_INSTALL_LIBSTDCPP > depends on BR2_TOOLCHAIN_GCC_AT_LEAST_7 # c++17 > # z3 needs fenv.h which is not provided by uclibc > depends on !BR2_TOOLCHAIN_USES_UCLIBC > - depends on BR2_PACKAGE_Z3_ARCH_SUPPORTS > + depends on BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING > help > Z3, also known as the Z3 Theorem Prover, is a cross-platform > satisfiability modulo theories (SMT) solver.
Hi Giulio, Le 09/02/2023 à 21:54, Giulio Benetti a écrit : > Hi Julien, > > On 09/02/23 20:36, Julien Olivain wrote: >> >> Hi Giulio, >> >> On 09/02/2023 01:48, Giulio Benetti wrote: >>> Package quickjs fails to build on architectures that don't support full >>> float rounding. To support all kind of float rounding FE_DOWNWARD, >>> FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. >>> Since package z3 locally implements this check let's make it global, >>> rename it to be generic and use it for both package quickjs and z3. >>> >>> Fixes: >>> http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ >>> http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ >>> http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ >>> http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ >>> >>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> >> >> To comment a bit on that, the complete condition is a bit more tricky: >> - the CPU arch FPU HW must have all 4 rounding modes, >> - the libc must expose a fenv.h header, >> - the libc must also expose all 4 rouding modes. > > thank you for the more precise explanation, everything is more clear now > >> glibc generally exposes all the HW supported rounding modes. I >> initially excluded uclibc-ng, because in its Buildroot default >> configuration, fenv.h is not enabled [1]. I believe it's possible to >> have some uclibc-ng configuration that are enable and exposing those >> rounding modes. See [2]. >> >> So this proposal is correct but possibly discard the case in which libc is >> uclibc-ng and its (custom) configuration exposes all FE rounding modes. > > Maybe we can enable UCLIBC_HAS_FENV on uclibc-ng by default, so all > architectures that UCLIBC_HAS_FLOAT they have it. Otherwise that UCLIBC_HAS_FENV > gets discarded if I'm correct. > >> I am not sure if it's really an issue, as packages with such >> requirements are mostly tested with glibc anyway... > > Same here for the architecture. > > So we'd need a valid condition that is true for the 3 points you've > listed above exactly in the order you've listed them if I'm correct. > > Maybe we can do an intermediate step with this patch and later deal with > this matrix validity check As discussed on IRC, I marked this patch as changes requested. Best regards, Romain > > Best regards
diff --git a/arch/Config.in b/arch/Config.in index 1c0c400a98..90846861ca 100644 --- a/arch/Config.in +++ b/arch/Config.in @@ -253,6 +253,30 @@ config BR2_xtensa endchoice +# Not all architectures support float rounding. This can be found while +# checking if fenv.h provides all four macros: +# FE_DOWNWARD, FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD +# See for example in glibc https://sourceware.org/git/glibc.git +# git grep -E '^[[:space:]]*#[[:space:]]*define[[:space:]]+FE_(TONEAREST|UPWARD|DOWNWARD|TOWARDZERO)' sysdeps/ +config BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING + bool + default y if BR2_aarch64 || BR2_aarch64_be + default y if BR2_arceb || BR2_arcle + default y if BR2_arm || BR2_armeb + default y if BR2_i386 + default y if BR2_m68k + # BR2_microblaze has only FE_TONEAREST + default y if BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el + # BR2_nios2 has only FE_TONEAREST + default y if BR2_or1k + default y if BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le + default y if BR2_riscv + default y if BR2_s390x + # BR2_sh has only FE_{TONEAREST,TOWARDZERO} + default y if BR2_sparc || BR2_sparc64 + default y if BR2_x86_64 + # BR2_xtensa supports only uclibc which does not have fenv.h + # For some architectures or specific cores, our internal toolchain # backend is not suitable (like, missing support in upstream gcc, or # no ChipCo fork exists...) diff --git a/package/quickjs/Config.in b/package/quickjs/Config.in index dc466241ba..6f24787333 100644 --- a/package/quickjs/Config.in +++ b/package/quickjs/Config.in @@ -1,6 +1,6 @@ config BR2_PACKAGE_QUICKJS bool "quickjs" - depends on !BR2_nios2 # fenv.h lacks FE_{DOWN,UP}WARD on nios2 + depends on BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING # fenv.h lacks FE_{DOWN,UP}WARD depends on !BR2_STATIC_LIBS # No way to check for fenv support. depends on !BR2_TOOLCHAIN_USES_UCLIBC @@ -15,7 +15,7 @@ config BR2_PACKAGE_QUICKJS https://bellard.org/quickjs/ comment "quickjs needs a glibc or musl toolchain w/ gcc >= 4.9, host gcc >= 4.9, dynamic library" - depends on !BR2_nios2 + depends on BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING depends on BR2_USE_MMU depends on BR2_STATIC_LIBS || BR2_TOOLCHAIN_USES_UCLIBC || \ !BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 || !BR2_HOST_GCC_AT_LEAST_4_9 diff --git a/package/z3/Config.in b/package/z3/Config.in index 8cd3128687..5ceca5dbe2 100644 --- a/package/z3/Config.in +++ b/package/z3/Config.in @@ -1,33 +1,10 @@ -# z3 supports arch for which libc fenv.h provides all four macros: -# FE_DOWNWARD, FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD -# See for example in glibc https://sourceware.org/git/glibc.git -# git grep -E '^[[:space:]]*#[[:space:]]*define[[:space:]]+FE_(TONEAREST|UPWARD|DOWNWARD|TOWARDZERO)' sysdeps/ -config BR2_PACKAGE_Z3_ARCH_SUPPORTS - bool - default y if BR2_aarch64 || BR2_aarch64_be - default y if BR2_arceb || BR2_arcle - default y if BR2_arm || BR2_armeb - default y if BR2_i386 - default y if BR2_m68k - # BR2_microblaze has only FE_TONEAREST - default y if BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el - # BR2_nios2 has only FE_TONEAREST - default y if BR2_or1k - default y if BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le - default y if BR2_riscv - default y if BR2_s390x - # BR2_sh has only FE_{TONEAREST,TOWARDZERO} - default y if BR2_sparc || BR2_sparc64 - default y if BR2_x86_64 - # BR2_xtensa supports only uclibc which does not have fenv.h - config BR2_PACKAGE_Z3 bool "z3" depends on BR2_INSTALL_LIBSTDCPP depends on BR2_TOOLCHAIN_GCC_AT_LEAST_7 # c++17 # z3 needs fenv.h which is not provided by uclibc depends on !BR2_TOOLCHAIN_USES_UCLIBC - depends on BR2_PACKAGE_Z3_ARCH_SUPPORTS + depends on BR2_PACKAGE_ARCH_HAS_ALL_FLOAT_ROUNDING help Z3, also known as the Z3 Theorem Prover, is a cross-platform satisfiability modulo theories (SMT) solver.
Package quickjs fails to build on architectures that don't support full float rounding. To support all kind of float rounding FE_DOWNWARD, FE_TONEAREST, FE_TOWARDZERO, FE_UPWARD must be defined in fenv.h. Since package z3 locally implements this check let's make it global, rename it to be generic and use it for both package quickjs and z3. Fixes: http://autobuild.buildroot.net/results/0b6/0b67381e3643c134c92cbcd1c5dc258fd760cd7e/ http://autobuild.buildroot.net/results/cf2/cf249a82c83d3c8503205f4c3c5859dd9d52afdf/ http://autobuild.buildroot.net/results/9b3/9b37c6c55a5b468657af26ef9b57d4fe24d8fde2/ http://autobuild.buildroot.net/results/4f1/4f13cf7d353260c6b8934dbdcf267bea884b356b/ Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> --- arch/Config.in | 24 ++++++++++++++++++++++++ package/quickjs/Config.in | 4 ++-- package/z3/Config.in | 25 +------------------------ 3 files changed, 27 insertions(+), 26 deletions(-)