[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[yegorslists at googlemail.com]

From: Yegor Yefremov <yegorslists@googlemail.com>

glibc fixed a bug, that defined FPU capabilities not
available for MIPS without FPU. So add --no-fpenvironment
to the configuration options for this case.

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v1 -> v2:
   - put all conditions in one ifeq statement (Thomas Petazzoni)
	
 package/poco/poco.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/poco/poco.mk b/package/poco/poco.mk
index 018344d5c7..af41d36ff7 100644
--- a/package/poco/poco.mk
+++ b/package/poco/poco.mk
@@ -42,6 +42,11 @@ ifeq ($(BR2_sh4a)$(BR2_nios2),y)
 POCO_CONF_OPTS += --no-fpenvironment
 endif
 
+# MIPS without FPU
+ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el):$(BR2_MIPS_SOFT_FLOAT),y:y)
+POCO_CONF_OPTS += --no-fpenvironment
+endif
+
 ifeq ($(BR2_STATIC_LIBS),y)
 POCO_MAKE_TARGET = static_release
 else ifeq ($(BR2_SHARED_LIBS),y)
-- 
2.17.0

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Arnout Vandecappelle]

On 9/10/18 15:21, yegorslists at googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> glibc fixed a bug, that defined FPU capabilities not
> available for MIPS without FPU. So add --no-fpenvironment
> to the configuration options for this case.

 The commit message is still very vague. I think I understand it though,
something like:

On MIPS soft-float, glibc defines some FPU capabilities in fpenv that don't
actually exist. This is a bug that has been fixed in glibc but not yet released.
Therefore, pass --no-fpenvironment to disable floating-point operations entirely.

 It would also be nice to point to the glibc bug.

> 
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> Changes v1 -> v2:
>    - put all conditions in one ifeq statement (Thomas Petazzoni)
> 	
>  package/poco/poco.mk | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/package/poco/poco.mk b/package/poco/poco.mk
> index 018344d5c7..af41d36ff7 100644
> --- a/package/poco/poco.mk
> +++ b/package/poco/poco.mk
> @@ -42,6 +42,11 @@ ifeq ($(BR2_sh4a)$(BR2_nios2),y)
>  POCO_CONF_OPTS += --no-fpenvironment
>  endif
>  
> +# MIPS without FPU
> +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el):$(BR2_MIPS_SOFT_FLOAT),y:y)

 BR2_MIPS_SOFT_FLOAT is only defined on MIPS, so it is sufficient to do

ifeq ($(BR2_MIPS_SOFT_FLOAT),y)

 However, since this is about a glibc bug, shouldn't it check for a glibc
toolchain as well?

 Regards,
 Arnout

> +POCO_CONF_OPTS += --no-fpenvironment
> +endif
> +
>  ifeq ($(BR2_STATIC_LIBS),y)
>  POCO_MAKE_TARGET = static_release
>  else ifeq ($(BR2_SHARED_LIBS),y)
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Thomas Petazzoni]

Hello,

On Tue, 9 Oct 2018 22:13:10 +0200, Arnout Vandecappelle wrote:

>  The commit message is still very vague. I think I understand it though,
> something like:

Yes, I agree the commit log was still too vague.

> > +# MIPS without FPU
> > +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el):$(BR2_MIPS_SOFT_FLOAT),y:y)  
> 
>  BR2_MIPS_SOFT_FLOAT is only defined on MIPS, so it is sufficient to do
> 
> ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
> 
>  However, since this is about a glibc bug, shouldn't it check for a glibc
> toolchain as well?

Isn't the glibc bug that it allowed to use fpenv in situations where it
shouldn't have allowed it ? If that's the case, perhaps the other
libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
soft-float, regardless of the C library in use ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Yegor Yefremov]

On Wed, Oct 10, 2018 at 8:45 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Tue, 9 Oct 2018 22:13:10 +0200, Arnout Vandecappelle wrote:
>
> >  The commit message is still very vague. I think I understand it though,
> > something like:
>
> Yes, I agree the commit log was still too vague.
>
> > > +# MIPS without FPU
> > > +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el):$(BR2_MIPS_SOFT_FLOAT),y:y)
> >
> >  BR2_MIPS_SOFT_FLOAT is only defined on MIPS, so it is sufficient to do
> >
> > ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
> >
> >  However, since this is about a glibc bug, shouldn't it check for a glibc
> > toolchain as well?
>
> Isn't the glibc bug that it allowed to use fpenv in situations where it
> shouldn't have allowed it ? If that's the case, perhaps the other
> libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
> soft-float, regardless of the C library in use ?

I've made a compile test with musl and it fails too. So I'll just
disable fpenv if BR2_MIPS_SOFT_FLOAT is selected. So ne need to
mention glibc.

Strange, that it didn't show up in the build tests ...

Yegor

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Yegor Yefremov]

On Wed, Oct 10, 2018 at 9:02 AM Yegor Yefremov
<yegorslists@googlemail.com> wrote:
>
> On Wed, Oct 10, 2018 at 8:45 AM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > Hello,
> >
> > On Tue, 9 Oct 2018 22:13:10 +0200, Arnout Vandecappelle wrote:
> >
> > >  The commit message is still very vague. I think I understand it though,
> > > something like:
> >
> > Yes, I agree the commit log was still too vague.
> >
> > > > +# MIPS without FPU
> > > > +ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el):$(BR2_MIPS_SOFT_FLOAT),y:y)
> > >
> > >  BR2_MIPS_SOFT_FLOAT is only defined on MIPS, so it is sufficient to do
> > >
> > > ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
> > >
> > >  However, since this is about a glibc bug, shouldn't it check for a glibc
> > > toolchain as well?
> >
> > Isn't the glibc bug that it allowed to use fpenv in situations where it
> > shouldn't have allowed it ? If that's the case, perhaps the other
> > libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
> > soft-float, regardless of the C library in use ?
>
> I've made a compile test with musl and it fails too. So I'll just
> disable fpenv if BR2_MIPS_SOFT_FLOAT is selected. So ne need to
> mention glibc.
>
> Strange, that it didn't show up in the build tests ...

For now we disable fpenv for two archs:

# architectures missing some FE_* in their fenv.h
ifeq ($(BR2_sh4a)$(BR2_nios2),y)
POCO_CONF_OPTS += --no-fpenvironment
endif

Should I just add $(BR2_MIPS_SOFT_FLOAT) to this statement even
without changing the comment?

Yegor

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Thomas Petazzoni]

Hello,

On Wed, 10 Oct 2018 09:02:02 +0200, Yegor Yefremov wrote:

> > Isn't the glibc bug that it allowed to use fpenv in situations where it
> > shouldn't have allowed it ? If that's the case, perhaps the other
> > libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
> > soft-float, regardless of the C library in use ?  
> 
> I've made a compile test with musl and it fails too. So I'll just
> disable fpenv if BR2_MIPS_SOFT_FLOAT is selected. So ne need to
> mention glibc.
> 
> Strange, that it didn't show up in the build tests ...

We don't test mips/musl in the autobuilders. However, we do test
mips/uclibc, but maybe not in soft-float mode.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Yegor Yefremov]

On Wed, Oct 10, 2018 at 9:12 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Wed, 10 Oct 2018 09:02:02 +0200, Yegor Yefremov wrote:
>
> > > Isn't the glibc bug that it allowed to use fpenv in situations where it
> > > shouldn't have allowed it ? If that's the case, perhaps the other
> > > libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
> > > soft-float, regardless of the C library in use ?
> >
> > I've made a compile test with musl and it fails too. So I'll just
> > disable fpenv if BR2_MIPS_SOFT_FLOAT is selected. So ne need to
> > mention glibc.
> >
> > Strange, that it didn't show up in the build tests ...
>
> We don't test mips/musl in the autobuilders. However, we do test
> mips/uclibc, but maybe not in soft-float mode.

uclibc has special handling: both fpenv and wstring will be disabled.

Yegor

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Arnout Vandecappelle]

On 10/10/18 09:14, Yegor Yefremov wrote:
> On Wed, Oct 10, 2018 at 9:12 AM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
>>
>> Hello,
>>
>> On Wed, 10 Oct 2018 09:02:02 +0200, Yegor Yefremov wrote:
>>
>>>> Isn't the glibc bug that it allowed to use fpenv in situations where it
>>>> shouldn't have allowed it ? If that's the case, perhaps the other
>>>> libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
>>>> soft-float, regardless of the C library in use ?
>>>
>>> I've made a compile test with musl and it fails too. So I'll just
>>> disable fpenv if BR2_MIPS_SOFT_FLOAT is selected. So ne need to
>>> mention glibc.
>>>
>>> Strange, that it didn't show up in the build tests ...
>>
>> We don't test mips/musl in the autobuilders. However, we do test
>> mips/uclibc, but maybe not in soft-float mode.
> 
> uclibc has special handling: both fpenv and wstring will be disabled.

 So, to be fully correct, poco should be build-tested and preferably
runtime-tested on all these softfloat architectures.

 Wouldn't it be a lot simpler to just disable the whole fpenv stuff on soft
float unconditionally? Yes, this is not perfect, because there are a few cases
where it actually *does* work. But if someone needs those cases, let them please
write a proper configure script that detects them. Until then, disable the
entire feature on soft float.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] poco: disable fpenvironment for MIPS without FPU

[Yegor Yefremov]

On Wed, Oct 10, 2018 at 9:45 AM Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 10/10/18 09:14, Yegor Yefremov wrote:
> > On Wed, Oct 10, 2018 at 9:12 AM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> >>
> >> Hello,
> >>
> >> On Wed, 10 Oct 2018 09:02:02 +0200, Yegor Yefremov wrote:
> >>
> >>>> Isn't the glibc bug that it allowed to use fpenv in situations where it
> >>>> shouldn't have allowed it ? If that's the case, perhaps the other
> >>>> libcs didn't had this bug, and fpenv really shouldn't be used with MIPS
> >>>> soft-float, regardless of the C library in use ?
> >>>
> >>> I've made a compile test with musl and it fails too. So I'll just
> >>> disable fpenv if BR2_MIPS_SOFT_FLOAT is selected. So ne need to
> >>> mention glibc.
> >>>
> >>> Strange, that it didn't show up in the build tests ...
> >>
> >> We don't test mips/musl in the autobuilders. However, we do test
> >> mips/uclibc, but maybe not in soft-float mode.
> >
> > uclibc has special handling: both fpenv and wstring will be disabled.
>
>  So, to be fully correct, poco should be build-tested and preferably
> runtime-tested on all these softfloat architectures.
>
>  Wouldn't it be a lot simpler to just disable the whole fpenv stuff on soft
> float unconditionally? Yes, this is not perfect, because there are a few cases
> where it actually *does* work. But if someone needs those cases, let them please
> write a proper configure script that detects them. Until then, disable the
> entire feature on soft float.

Patch already sent. I've tried ARM with soft floating point and it
also fails with enabled fpenvironment.

Yegor