Remove multiple defination error

Message ID 20200710224109.18663-1-eshandhawan51@gmail.com
State New
Headers show
Series
  • Remove multiple defination error
Related show

Commit Message

Eshan dhawan via Newlib July 10, 2020, 10:41 p.m.
Signed-off-by: Eshan dhawan <eshandhawan51@gmail.com>

---
 newlib/libm/machine/arm/fenv-vfp.c |  7 ++++-
 newlib/libm/machine/arm/fenv.c     | 47 +++++++++++++++++++-----------
 2 files changed, 36 insertions(+), 18 deletions(-)

-- 
2.17.1

Comments

Sebastian Huber July 11, 2020, 3:16 p.m. | #1
On 11/07/2020 00:41, Eshan dhawan wrote:

> Signed-off-by: Eshan dhawan<eshandhawan51@gmail.com>

> ---

>   newlib/libm/machine/arm/fenv-vfp.c |  7 ++++-

>   newlib/libm/machine/arm/fenv.c     | 47 +++++++++++++++++++-----------

>   2 files changed, 36 insertions(+), 18 deletions(-)


What are the design goals of this implementation? Should it work with 
some sort of run time VFP detection? Should it select the support using 
builtin defines?

I would use builtin defines only and then just use a single file.
Eshan dhawan via Newlib July 11, 2020, 5:18 p.m. | #2
On Sat, Jul 11, 2020 at 8:46 PM Sebastian Huber <
sebastian.huber@embedded-brains.de> wrote:

> On 11/07/2020 00:41, Eshan dhawan wrote:

>

> > Signed-off-by: Eshan dhawan<eshandhawan51@gmail.com>

> > ---

> >   newlib/libm/machine/arm/fenv-vfp.c |  7 ++++-

> >   newlib/libm/machine/arm/fenv.c     | 47 +++++++++++++++++++-----------

> >   2 files changed, 36 insertions(+), 18 deletions(-)

>

> What are the design goals of this implementation? Should it work with

> some sort of run time VFP detection? Should it select the support using

> builtin defines?

>

It used the GCC flags to detect VPF
if VPF is not selected all the symbols of vfp won't appear :)

>

> I would use builtin defines only and then just use a single file.

>

I tried converting into a single file but the file becomes too complex to
handle and check for errors. :)
Sebastian Huber July 13, 2020, 5:36 a.m. | #3
On 11/07/2020 19:18, Eshan Dhawan wrote:
> 

> 

> On Sat, Jul 11, 2020 at 8:46 PM Sebastian Huber 

> <sebastian.huber@embedded-brains.de 

> <mailto:sebastian.huber@embedded-brains.de>> wrote:

> 

>     On 11/07/2020 00:41, Eshan dhawan wrote:

> 

>      > Signed-off-by: Eshan dhawan<eshandhawan51@gmail.com

>     <mailto:eshandhawan51@gmail.com>>

>      > ---

>      >   newlib/libm/machine/arm/fenv-vfp.c |  7 ++++-

>      >   newlib/libm/machine/arm/fenv.c     | 47

>     +++++++++++++++++++-----------

>      >   2 files changed, 36 insertions(+), 18 deletions(-)

> 

>     What are the design goals of this implementation? Should it work with

>     some sort of run time VFP detection? Should it select the support using

>     builtin defines?

> 

> It used the GCC flags to detect VPF

> if VPF is not selected all the symbols of vfp won't appear :)

> 

> 

>     I would use builtin defines only and then just use a single file.

> 

> I tried converting into a single file but the file becomes too complex 

> to handle and check for errors. :)


Complexity is always a bit subjective.

How did you test the ARM implementation? I doubt the soft-float support 
works at all. FreeBSD ships its own soft-float library. Newlib uses the 
one from libgcc by default. They are not compatible.

I guess the MIPS soft-float fenv support has this issue as well.

Having an GCC incompatible soft-float fenv support in Newlib makes no 
sense to me. How can we fix this? Disable it completely or return some 
run time errors?

Maybe we should remove the ARM fenv support from Newlib until these 
issues are fixed and a working implementation is available.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Sebastian Huber July 13, 2020, 5:48 a.m. | #4
On 13/07/2020 07:36, Sebastian Huber wrote:
> On 11/07/2020 19:18, Eshan Dhawan wrote:

>>

>>

>> On Sat, Jul 11, 2020 at 8:46 PM Sebastian Huber 

>> <sebastian.huber@embedded-brains.de 

>> <mailto:sebastian.huber@embedded-brains.de>> wrote:

>>

>>     On 11/07/2020 00:41, Eshan dhawan wrote:

>>

>>      > Signed-off-by: Eshan dhawan<eshandhawan51@gmail.com

>>     <mailto:eshandhawan51@gmail.com>>

>>      > ---

>>      >   newlib/libm/machine/arm/fenv-vfp.c |  7 ++++-

>>      >   newlib/libm/machine/arm/fenv.c     | 47

>>     +++++++++++++++++++-----------

>>      >   2 files changed, 36 insertions(+), 18 deletions(-)

>>

>>     What are the design goals of this implementation? Should it work with

>>     some sort of run time VFP detection? Should it select the support 

>> using

>>     builtin defines?

>>

>> It used the GCC flags to detect VPF

>> if VPF is not selected all the symbols of vfp won't appear :)

>>

>>

>>     I would use builtin defines only and then just use a single file.

>>

>> I tried converting into a single file but the file becomes too complex 

>> to handle and check for errors. :)

> 

> Complexity is always a bit subjective.

> 

> How did you test the ARM implementation? I doubt the soft-float support 

> works at all. FreeBSD ships its own soft-float library. Newlib uses the 

> one from libgcc by default. They are not compatible.

> 

> I guess the MIPS soft-float fenv support has this issue as well.

> 

> Having an GCC incompatible soft-float fenv support in Newlib makes no 

> sense to me. How can we fix this? Disable it completely or return some 

> run time errors?

> 

> Maybe we should remove the ARM fenv support from Newlib until these 

> issues are fixed and a working implementation is available.


I noticed that the PowerPC fenv support unconditionally assumes that a 
floating-point unit is available. Not all PowerPC processors support a 
floating-point unit and some applications may intentionally use 
soft-float. Using this code may result in an unimplemented exception if 
MSR[FP]=1, or an FP Unavailable exception if MSR[FP]=0.

If this is a desirable behaviour, then we can use the same on ARM. Just 
provide the VFP implementation. If you use the stuff in other setups you 
get an exception.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Joel Sherrill July 14, 2020, 1:11 p.m. | #5
On Mon, Jul 13, 2020 at 12:48 AM Sebastian Huber <
sebastian.huber@embedded-brains.de> wrote:

> On 13/07/2020 07:36, Sebastian Huber wrote:

> > On 11/07/2020 19:18, Eshan Dhawan wrote:

> >>

> >>

> >> On Sat, Jul 11, 2020 at 8:46 PM Sebastian Huber

> >> <sebastian.huber@embedded-brains.de

> >> <mailto:sebastian.huber@embedded-brains.de>> wrote:

> >>

> >>     On 11/07/2020 00:41, Eshan dhawan wrote:

> >>

> >>      > Signed-off-by: Eshan dhawan<eshandhawan51@gmail.com

> >>     <mailto:eshandhawan51@gmail.com>>

> >>      > ---

> >>      >   newlib/libm/machine/arm/fenv-vfp.c |  7 ++++-

> >>      >   newlib/libm/machine/arm/fenv.c     | 47

> >>     +++++++++++++++++++-----------

> >>      >   2 files changed, 36 insertions(+), 18 deletions(-)

> >>

> >>     What are the design goals of this implementation? Should it work

> with

> >>     some sort of run time VFP detection? Should it select the support

> >> using

> >>     builtin defines?

> >>

> >> It used the GCC flags to detect VPF

> >> if VPF is not selected all the symbols of vfp won't appear :)

> >>

> >>

> >>     I would use builtin defines only and then just use a single file.

> >>

> >> I tried converting into a single file but the file becomes too complex

> >> to handle and check for errors. :)

> >

> > Complexity is always a bit subjective.

> >

> > How did you test the ARM implementation? I doubt the soft-float support

> > works at all. FreeBSD ships its own soft-float library. Newlib uses the

> > one from libgcc by default. They are not compatible.

> >

> > I guess the MIPS soft-float fenv support has this issue as well.

> >

> > Having an GCC incompatible soft-float fenv support in Newlib makes no

> > sense to me. How can we fix this? Disable it completely or return some

> > run time errors?

> >

> > Maybe we should remove the ARM fenv support from Newlib until these

> > issues are fixed and a working implementation is available.

>

> I noticed that the PowerPC fenv support unconditionally assumes that a

> floating-point unit is available. Not all PowerPC processors support a

> floating-point unit and some applications may intentionally use

> soft-float. Using this code may result in an unimplemented exception if

> MSR[FP]=1, or an FP Unavailable exception if MSR[FP]=0.

>

> If this is a desirable behaviour, then we can use the same on ARM. Just

> provide the VFP implementation. If you use the stuff in other setups you

> get an exception.

>


The RISC-V would be a good model. Jim Wilson provided some default
behavior when there is no hard FPU.

--joel

>

> --

> Sebastian Huber, embedded brains GmbH

>

> Address : Dornierstr. 4, D-82178 Puchheim, Germany

> Phone   : +49 89 189 47 41-16

> Fax     : +49 89 189 47 41-09

> E-Mail  : sebastian.huber@embedded-brains.de

> PGP     : Public key available on request.

>

> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

>
Jim Wilson July 14, 2020, 4:59 p.m. | #6
On Tue, Jul 14, 2020 at 6:11 AM Joel Sherrill <joel@rtems.org> wrote:
> The RISC-V would be a good model. Jim Wilson provided some default

> behavior when there is no hard FPU.


It wasn't me personally.  This predates my involvement.  I'd guess
either Andrew Waterman, Palmer Dabbelt, or Kito Cheng.  Anyways, we
use the hard FPU if available, both for hard-float and soft-float
ABIs.  If no hard FPU, then the various fenv routines do very little,
just returning some default values.

Jim
Sebastian Huber July 14, 2020, 5:51 p.m. | #7
On 14/07/2020 18:59, Jim Wilson wrote:

> On Tue, Jul 14, 2020 at 6:11 AM Joel Sherrill<joel@rtems.org>  wrote:

>> The RISC-V would be a good model. Jim Wilson provided some default

>> behavior when there is no hard FPU.

> It wasn't me personally.  This predates my involvement.  I'd guess

> either Andrew Waterman, Palmer Dabbelt, or Kito Cheng.  Anyways, we

> use the hard FPU if available, both for hard-float and soft-float

> ABIs.  If no hard FPU, then the various fenv routines do very little,

> just returning some default values.


Ok, good.

Eshan, could you please use the RISC-V as a model for the ARM support. 
Please reduce the file count. I think we just need one fenv.c and not 
additional source and header files. Alternatively, you can split it up 
into one file per function similar to RISC-V.

Patch

diff --git a/newlib/libm/machine/arm/fenv-vfp.c b/newlib/libm/machine/arm/fenv-vfp.c
index 297e81296..11a05ea54 100644
--- a/newlib/libm/machine/arm/fenv-vfp.c
+++ b/newlib/libm/machine/arm/fenv-vfp.c
@@ -26,7 +26,12 @@ 
  * $FreeBSD$
  */
 
+#if defined(__VFP_FP__) && !defined(__ARM_PCS_VFP)
+#define SOFTFP_ABI
+#endif
+
+#ifndef SOFTFP_ABI
 #define	FENV_MANGLE(x)	__vfp_ ##x
 #include <machine/fenv-mangle.h>
 #include "fenv.c"
-
+#endif
diff --git a/newlib/libm/machine/arm/fenv.c b/newlib/libm/machine/arm/fenv.c
index 5b61ab81f..98125cacf 100644
--- a/newlib/libm/machine/arm/fenv.c
+++ b/newlib/libm/machine/arm/fenv.c
@@ -43,7 +43,6 @@ 
 #define SOFTFP_ABI
 #endif
 
-
 #ifndef FENV_MANGLE
 /*
  * Hopefully the system ID byte is immutable, so it's valid to use
@@ -102,8 +101,6 @@  extern inline int fegetexcept(void);
 #else /* !FENV_MANGLE && SOFTFP_ABI */
 /* Set by libc when the VFP unit is enabled */
 
-int _libc_arm_fpu_present;
-
 int __softfp_feclearexcept(int excepts);
 int __softfp_fegetexceptflag(fexcept_t *flagp, int excepts);
 int __softfp_fesetexceptflag(const fexcept_t *flagp, int excepts);
@@ -119,6 +116,7 @@  int __softfp_feenableexcept(int __mask);
 int __softfp_fedisableexcept(int __mask);
 int __softfp_fegetexcept(void);
 
+#ifndef SOFTFP_ABI
 int __vfp_feclearexcept(int excepts);
 int __vfp_fegetexceptflag(fexcept_t *flagp, int excepts);
 int __vfp_fesetexceptflag(const fexcept_t *flagp, int excepts);
@@ -133,6 +131,7 @@  int __vfp_feupdateenv(const fenv_t *envp);
 int __vfp_feenableexcept(int __mask);
 int __vfp_fedisableexcept(int __mask);
 int __vfp_fegetexcept(void);
+#endif
 
 static int
 __softfp_round_to_vfp(int round)
@@ -171,8 +170,9 @@  __softfp_round_from_vfp(int round)
 int feclearexcept(int excepts)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_feclearexcept(excepts);
+#endif
 	__softfp_feclearexcept(excepts);
 
 	return (0);
@@ -183,8 +183,9 @@  int fegetexceptflag(fexcept_t *flagp, int excepts)
 	fexcept_t __vfp_flagp;
 
 	__vfp_flagp = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_fegetexceptflag(&__vfp_flagp, excepts);
+#endif
 	__softfp_fegetexceptflag(flagp, excepts);
 
 	*flagp |= __vfp_flagp;
@@ -195,8 +196,9 @@  int fegetexceptflag(fexcept_t *flagp, int excepts)
 int fesetexceptflag(const fexcept_t *flagp, int excepts)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_fesetexceptflag(flagp, excepts);
+#endif
 	__softfp_fesetexceptflag(flagp, excepts);
 
 	return (0);
@@ -205,8 +207,9 @@  int fesetexceptflag(const fexcept_t *flagp, int excepts)
 int feraiseexcept(int excepts)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_feraiseexcept(excepts);
+#endif
 	__softfp_feraiseexcept(excepts);
 
 	return (0);
@@ -217,8 +220,9 @@  int fetestexcept(int excepts)
 	int __got_excepts;
 
 	__got_excepts = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__got_excepts = __vfp_fetestexcept(excepts);
+#endif
 	__got_excepts |= __softfp_fetestexcept(excepts);
 
 	return (__got_excepts);
@@ -227,16 +231,18 @@  int fetestexcept(int excepts)
 int fegetround(void)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		return __softfp_round_from_vfp(__vfp_fegetround());
+#endif
 	return __softfp_fegetround();
 }
 
 int fesetround(int round)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_fesetround(__softfp_round_to_vfp(round));
+#endif
 	__softfp_fesetround(round);
 
 	return (0);
@@ -247,8 +253,9 @@  int fegetenv(fenv_t *envp)
 	fenv_t __vfp_envp;
 
 	__vfp_envp = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_fegetenv(&__vfp_envp);
+#endif
 	__softfp_fegetenv(envp);
 	*envp |= __vfp_envp;
 
@@ -260,8 +267,9 @@  int feholdexcept(fenv_t *envp)
 	fenv_t __vfp_envp;
 
 	__vfp_envp = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_feholdexcept(&__vfp_envp);
+#endif
 	__softfp_feholdexcept(envp);
 	*envp |= __vfp_envp;
 
@@ -271,8 +279,9 @@  int feholdexcept(fenv_t *envp)
 int fesetenv(const fenv_t *envp)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_fesetenv(envp);
+#endif
 	__softfp_fesetenv(envp);
 
 	return (0);
@@ -281,8 +290,9 @@  int fesetenv(const fenv_t *envp)
 int feupdateenv(const fenv_t *envp)
 {
 
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__vfp_feupdateenv(envp);
+#endif
 	__softfp_feupdateenv(envp);
 
 	return (0);
@@ -293,8 +303,9 @@  int feenableexcept(int __mask)
 	int __unmasked;
 
 	__unmasked = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__unmasked = __vfp_feenableexcept(__mask);
+#endif
 	__unmasked |= __softfp_feenableexcept(__mask);
 
 	return (__unmasked);
@@ -305,8 +316,9 @@  int fedisableexcept(int __mask)
 	int __unmasked;
 
 	__unmasked = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__unmasked = __vfp_fedisableexcept(__mask);
+#endif
 	__unmasked |= __softfp_fedisableexcept(__mask);
 
 	return (__unmasked);
@@ -317,8 +329,9 @@  int fegetexcept(void)
 	int __unmasked;
 
 	__unmasked = 0;
-	if (_libc_arm_fpu_present)
+#ifndef SOFTFP_ABI
 		__unmasked = __vfp_fegetexcept();
+#endif
 	__unmasked |= __softfp_fegetexcept();
 
 	return (__unmasked);