This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, ARMEL, FPA] revert-arm-ieee-word-endian.patch - correct newlib-1.18.0 soft FP breakage


Hello Ralf and other,

there is fundamental miscompilation problem for libm floating-point functions
(sin, cos etc.) for default (non-eabi/VFP) build. The problem demonstreted
to us when we have build GCC-4.4.4 with NewLib-1.18.0 but it should be same
for all GCC versions and NewLib after next patch

2009-12-17  Ralf Corsepius

        * libc/include/machine/ieeefp.h: Rework __IEEE_*_ENDIAN handling.
        * libc/machine/arm/machine/endian.h: Remove (Conflicts with
        libc/include/machine/endian.h)

I am sending analysis of the problem and possible solutions and providing patch
based on relatively not so much intrusive changes.

The problem is, that , that if FPA or simple --with-float=soft configuration
is used for GCC, the double format floating points numbers are stored in memory
and passed to the functions in most significant word first format independently
of selected memory access endianness - ARMEL, ARMEB

  src/gcc-4.4/gcc/config/arm/arm.c:arm_float_words_big_endian() -> 1

This means, that for ARMEL (little endian) builds the next defines
has to be enabled in src/newlib/libc/include/machine/ieeefp.h

 #define __IEEE_BIG_ENDIAN
 #define __IEEE_BYTES_LITTLE_ENDIAN

These defines are crucial to agree libm high/low word extraction
macros in (enewlib/src/newlib/libm/common/fdlibm.h EXTRACT_WORDS etc)
with words order used by GCC code generator and libgcc.a.

But above the mentioned patch breaks this rule and fatal mitchmath
is observed. The above patch has been driven by attempt to solve
other/unrelated problem with integer/memory access endianness
which originates in src/newlib/libc/include/machine/endian.h .
This include file attempts to be architecture agnostic and that
is why it looks for endianness in sys/config.h (nothing there about endianness
fond in this file) which includes machine/ieeefp.h. BYTE_ORDER is not
found till that point and machine/endian.h decides to select endianness
according to __IEEE_BIG_ENDIAN which was wrong for ARMEL.
Ralf's change corrects this problem by switching to __IEEE_LITTLE_ENDIAN
for ARMEL, but that breaks floating point arithmetic so that change should
be reverted.

Patch below proposes to select BYTE_ORDER = BIG_ENDIAN only
if both __IEEE_LITTLE_ENDIAN and __IEEE_BYTES_LITTLE_ENDIAN are not set
which select right endianness in this case.

Other, more logical solution is to disconnect memory access endianness
selection from IEEE settings. In the fact, there is target architecture
specific/generated machine/param.h for ARM-ELF NEWLIB build
so its use as include in machine/endian.h would be logical. Problem
is, that this file sets more parameters then is expected from 
machine/endian.h. There is other problem, that I have seen some RTEMS version
of param.h header file which includes machine/endian.h file. This
could lead to include cycle problem. That is why, I propose the first
solution for now.

The other possibility is to consider regular arm-elf build without
-mfpu=vfp -mfloat-abi=soft (non EABI) as deprecated and left it broken.
But I do not consider this as a good choice to ship NewLib/GCC with
default build options broken.

There is full set of options for GCC-4.4.4 source tree configuration
with all bootstrap required components and system dependencies
symlinked into gcc-4.4.4 to exclude possible dependencies on broken/old
system provided MPFR and GMP packages
  gmp -> ../gmp-4.3.2
  mpfr -> ../mpfr-2.4.2
  newlib -> ../newlib-1.18.0/newlib

../../../src/gcc-4.4/configure -v \
         --enable-languages=c,c++ \
         --prefix=/usr \
         --enable-interwork \
         --enable-multilib \
         --with-system-zlib \
         --without-included-gettext \
         --disable-nls \
         --with-gnu-ld \
         --with-gnu-as \
         --with-newlib \
         --enable-checking=release \
         --build=x86_64-linux-gnu \
         --host=x86_64-linux-gnu \
         --target=arm-elf \
         --enable-version-specific-runtime-libs \
         --with-float=soft \
         --enable-target-optspace

We have build toolchain which compiles correctly soft-float code for applications
using non trivial (sin/cos) arithmetic functions that way. Tested in GDB sim
and on LPC23xx. 

The Debian packages for our internal use and CTU students are there

  http://rtime.felk.cvut.cz/hw/index.php/Cross_compilers
  ftp://rtime.felk.cvut.cz/debian/pool/arm-elf/

We still see some problems with NewLib printf for floating point numbers
after switching to the new toolchain version but this is probably
unrelated problem caused by changes in malloc integration into our board
or similar thing.

Best wishes,

                Pavel

                Pavel Pisa
    e-mail:     pisa@cmp.felk.cvut.cz
    www:        http://cmp.felk.cvut.cz/~pisa
    university: http://dce.felk.cvut.cz/
    company:    http://www.pikron.com/

---
 newlib/libc/include/machine/endian.h |    3 ++-
 newlib/libc/include/machine/ieeefp.h |    6 +-----
 2 files changed, 3 insertions(+), 6 deletions(-)

Index: newlib-1.18.0/newlib/libc/include/machine/ieeefp.h
===================================================================
--- newlib-1.18.0.orig/newlib/libc/include/machine/ieeefp.h	2010-06-15 22:46:23.000000000 +0000
+++ newlib-1.18.0/newlib/libc/include/machine/ieeefp.h	2010-06-15 22:50:53.000000000 +0000
@@ -62,12 +62,8 @@
 #  define __IEEE_BIG_ENDIAN
 # endif
 #else
+# define __IEEE_BIG_ENDIAN
 # ifdef __ARMEL__
-#  define __IEEE_LITTLE_ENDIAN
-# else
-#  define __IEEE_BIG_ENDIAN
-# endif
-# ifdef __ARMWEL__
 #  define __IEEE_BYTES_LITTLE_ENDIAN
 # endif
 #endif
Index: newlib-1.18.0/newlib/libc/include/machine/endian.h
===================================================================
--- newlib-1.18.0.orig/newlib/libc/include/machine/endian.h	2010-06-15 23:26:31.000000000 +0000
+++ newlib-1.18.0/newlib/libc/include/machine/endian.h	2010-06-15 23:22:17.000000000 +0000
@@ -10,7 +10,8 @@
 #endif
 
 #ifndef BYTE_ORDER
-#ifdef __IEEE_LITTLE_ENDIAN
+/* This whole thing is rotten but the next check should work on known architectures */
+#if defined(__IEEE_LITTLE_ENDIAN) || defined(__IEEE_BYTES_LITTLE_ENDIAN)
 #define BYTE_ORDER LITTLE_ENDIAN
 #else
 #define BYTE_ORDER BIG_ENDIAN


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]