Miscellaneous ColdFire patches

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Miscellaneous ColdFire patches

Joseph Myers
The attached patches are various glibc changes for ColdFire that have been
in EGLIBC for some years.  I've extracted the current versions of the
changes and generally reworked the ChangeLog entries, but note that I have
*not* tested any of these patches; they may however be useful to some
users.  Are any or all of them OK?

1. Define MMAP2_PAGE_SHIFT to 13, as previously discussed at
<http://sourceware.org/ml/libc-ports/2009-10/msg00005.html>.  The kernel
port (or at least there relevant part thereof) is now upstream, still with
this value.  Given the request last time to use getpagesize instead of a
hardcoded value, also attached is:

2. Variant of that patch, defining MMAP2_PAGE_SHIFT to -1 instead of 13
for ColdFire, so using the code now present in mmap64.c to get the value
using getpagesize.

3. Make fpu_control.h handle no-FPU ColdFire, in a similar way to how
several other architectures' fpu_control.h files handle soft-float cases.  
This patch doesn't use "# define" indentation, given that existing
conditionals in this file don't use it either.

4. Handle no-FPU ColdFire in dl-trampoline.S.

--
Joseph S. Myers
[hidden email]

glibc-cf-page-shift-1 (952 bytes) Download Attachment
glibc-cf-page-shift-2 (1K) Download Attachment
glibc-cf-fpu-control (2K) Download Attachment
glibc-cf-dl-trampoline (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Miscellaneous ColdFire patches

Andreas Schwab-2
"Joseph S. Myers" <[hidden email]> writes:

> 2013-06-28  Joseph Myers  <[hidden email]>
>
> * sysdeps/unix/sysv/linux/m68k/kernel-features.h [__mcoldfire__]
> (MMAP2_PAGE_SHIFT): Define to -1.

kernel-features.h is the wrong place to define this (mmap64.c should
actually no longer include it).  You should create a mmap64.c that
defines MMAP2_PAGE_SHIFT and then falls through to the real
implementation.  Also, this is an issue for sun3 as well, so it needs to
be defined for all m68k variants.

Finally, you need to fix sysdeps/unix/sysv/linux/mmap64.c so that it
doesn't shift by -1.

(Looks like cris and microblaze have the same bug.)

> 2013-06-28  Nathan Sidwell  <[hidden email]>
>
> * sysdeps/m68k/fpu_control.h [__mcoldfire__ && !__mcffpu__]
> (_FPU_RESERVED): Provide alternative definition.
> [__mcoldfire__ && !__mcffpu__] (_FPU_DEFAULT): Likewise.
> [__mcoldfire__ && !__mcffpu__] (_FPU_GETCW): Likewise.
> [__mcoldfire__ && !__mcffpu__] (_FPU_SETCW): Likewise.
> [!(__mcoldfire__ && !__mcffpu__)]: Make existing macro definitions
> conditional.
>
> diff --git a/ports/sysdeps/m68k/fpu_control.h b/ports/sysdeps/m68k/fpu_control.h
> index c37fcf4..848b5ba 100644
> --- a/ports/sysdeps/m68k/fpu_control.h
> +++ b/ports/sysdeps/m68k/fpu_control.h
> @@ -53,6 +53,15 @@
>  
>  #include <features.h>
>  
> +#if defined (__mcoldfire__) && !defined (__mcffpu__)
> +
> +#define _FPU_RESERVED 0xffffffff
> +#define _FPU_DEFAULT  0x00000000
> +#define _FPU_GETCW(cw) ((cw) = 0)
> +#define _FPU_SETCW(cw) ((void) (cw))
> +
> +#else
> +
>  /* masking of interrupts */
>  #define _FPU_MASK_BSUN  0x8000
>  #define _FPU_MASK_SNAN  0x4000
> @@ -95,12 +104,13 @@
>     that __setfpucw works.  This bit will be ignored.  */
>  #define _FPU_IEEE     0x00000001
>  
> -/* Type of the control word.  */
> -typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
> -
>  /* Macros for accessing the hardware control word.  */
>  #define _FPU_GETCW(cw) __asm__ ("fmove%.l %!, %0" : "=dm" (cw))
>  #define _FPU_SETCW(cw) __asm__ volatile ("fmove%.l %0, %!" : : "dm" (cw))
> +#endif
> +
> +/* Type of the control word.  */
> +typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
>  
>  /* Default control word set at startup.  */
>  extern fpu_control_t __fpu_control;

Please fix up the preprocessor indentation, possibly in a separate
commit.

> 2013-06-28  Nathan Sidwell  <[hidden email]>
>    Joseph Myers  <[hidden email]>
>
> * sysdeps/m68k/dl-trampoline.S (_dl_runtime_profile)
> [__mcoldfire__ && !__mcffpu]: Do not save floating-point
> registers.
>
> diff --git a/ports/sysdeps/m68k/dl-trampoline.S b/ports/sysdeps/m68k/dl-trampoline.S
> index 5aeafc7..16f20dc 100644
> --- a/ports/sysdeps/m68k/dl-trampoline.S
> +++ b/ports/sysdeps/m68k/dl-trampoline.S
> @@ -174,12 +174,16 @@ _dl_runtime_profile:
>      +4      %a1
>     %sp      %a0
>   */
> -#ifdef __mcoldfire__
> +#if !defined (__mcoldfire__)
> + fmove.x %fp0, -(%sp)
> + cfi_adjust_cfa_offset (12)
> +#elif defined (__mcffpu__)
>   fmove.d %fp0, -(%sp)
>   cfi_adjust_cfa_offset (8)
>  #else
> - fmove.x %fp0, -(%sp)
> - cfi_adjust_cfa_offset (12)
> + clr.l -(%sp)
> + clr.l -(%sp)
> + cfi_adjust_cfa_offset (8)

There should be no need to even allocate the stack space.

>  #endif
>   move.l %a0, -(%sp)
>   cfi_adjust_cfa_offset (4)
> @@ -213,15 +217,20 @@ _dl_runtime_profile:
>   cfi_adjust_cfa_offset (-4)
>   move.l (%sp)+, %a0
>   cfi_adjust_cfa_offset (-4)
> -#ifdef __mcoldfire__
> - fmove.d (%sp)+, %fp0
> - cfi_adjust_cfa_offset (-8)
> -#else
> +#if !defined (__mcoldfire__)
>   fmove.x (%sp)+, %fp0
>   cfi_adjust_cfa_offset (-12)
> -#endif
>   lea 20(%sp), %sp
>   cfi_adjust_cfa_offset (-20)
> +#elif defined (__mcffpu__)
> + fmove.l (%sp)+, %fp0

fmove.d

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: Miscellaneous ColdFire patches

Joseph Myers
On Fri, 28 Jun 2013, Andreas Schwab wrote:

> There should be no need to even allocate the stack space.

Here is a revised patch which avoids the stack allocation in this
case, using new macros FMOVE and FPSPACE to abstract the differences
between classic m68k, ColdFire with FPU, and ColdFire without FPU.
Again, completely untested.

2013-06-28  Joseph Myers  <[hidden email]>

        * sysdeps/m68k/dl-trampoline.S (FMOVE): Define conditional on
        [__mcoldfire__] and [__mcffpu__].
        (FPSPACE): Likewise.
        (_dl_runtime_profile): Save and restore %fp0 with FMOVE, only if
        [FMOVE].  Use FPSPACE in stack offsets.

diff --git a/ports/sysdeps/m68k/dl-trampoline.S b/ports/sysdeps/m68k/dl-trampoline.S
index 5aeafc7..a4caa67 100644
--- a/ports/sysdeps/m68k/dl-trampoline.S
+++ b/ports/sysdeps/m68k/dl-trampoline.S
@@ -18,6 +18,16 @@
 
 #include <sysdep.h>
 
+#if !defined (__mcoldfire__)
+# define FMOVE fmove.x
+# define FPSPACE 12
+#elif defined (__mcffpu__)
+# define FMOVE fmove.d
+# define FPSPACE 8
+#else
+# define FPSPACE 0
+#endif
+
  .text
  .globl _dl_runtime_resolve
  .type _dl_runtime_resolve, @function
@@ -174,12 +184,9 @@ _dl_runtime_profile:
     +4      %a1
    %sp      %a0
  */
-#ifdef __mcoldfire__
- fmove.d %fp0, -(%sp)
- cfi_adjust_cfa_offset (8)
-#else
- fmove.x %fp0, -(%sp)
- cfi_adjust_cfa_offset (12)
+#ifdef FMOVE
+ FMOVE %fp0, -(%sp)
+ cfi_adjust_cfa_offset (FPSPACE)
 #endif
  move.l %a0, -(%sp)
  cfi_adjust_cfa_offset (4)
@@ -189,21 +196,12 @@ _dl_runtime_profile:
  cfi_adjust_cfa_offset (4)
  pea (%sp)
  cfi_adjust_cfa_offset (4)
-#ifdef __mcoldfire__
- pea 24(%sp)
- cfi_adjust_cfa_offset (4)
- move.l 40(%sp), -(%sp)
+ pea (16+FPSPACE)(%sp)
  cfi_adjust_cfa_offset (4)
- move.l 40(%sp), -(%sp)
+ move.l (32+FPSPACE)(%sp), -(%sp)
  cfi_adjust_cfa_offset (4)
-#else
- pea 28(%sp)
- cfi_adjust_cfa_offset (4)
- move.l 44(%sp), -(%sp)
+ move.l (32+FPSPACE)(%sp), -(%sp)
  cfi_adjust_cfa_offset (4)
- move.l 44(%sp), -(%sp)
- cfi_adjust_cfa_offset (4)
-#endif
  jbsr _dl_call_pltexit
  lea 16(%sp), %sp
  cfi_adjust_cfa_offset (-16)
@@ -213,12 +211,9 @@ _dl_runtime_profile:
  cfi_adjust_cfa_offset (-4)
  move.l (%sp)+, %a0
  cfi_adjust_cfa_offset (-4)
-#ifdef __mcoldfire__
- fmove.d (%sp)+, %fp0
- cfi_adjust_cfa_offset (-8)
-#else
- fmove.x (%sp)+, %fp0
- cfi_adjust_cfa_offset (-12)
+#ifdef FMOVE
+ FMOVE (%sp)+, %fp0
+ cfi_adjust_cfa_offset (-FPSPACE)
 #endif
  lea 20(%sp), %sp
  cfi_adjust_cfa_offset (-20)


--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Miscellaneous ColdFire patches

Joseph Myers
In reply to this post by Andreas Schwab-2
On Fri, 28 Jun 2013, Andreas Schwab wrote:

> "Joseph S. Myers" <[hidden email]> writes:
>
> > 2013-06-28  Joseph Myers  <[hidden email]>
> >
> > * sysdeps/unix/sysv/linux/m68k/kernel-features.h [__mcoldfire__]
> > (MMAP2_PAGE_SHIFT): Define to -1.
>
> kernel-features.h is the wrong place to define this (mmap64.c should
> actually no longer include it).  You should create a mmap64.c that
> defines MMAP2_PAGE_SHIFT and then falls through to the real
> implementation.  Also, this is an issue for sun3 as well, so it needs to
> be defined for all m68k variants.
>
> Finally, you need to fix sysdeps/unix/sysv/linux/mmap64.c so that it
> doesn't shift by -1.
>
> (Looks like cris and microblaze have the same bug.)

This patch (untested) creates a new file and enables the change for
all m68k configurations.  I've sent the fix to
sysdeps/unix/sysv/linux/mmap64.c to libc-alpha and filed bug 15705 for
the MicroBlaze issue (the CRIS port of glibc having been removed as
bitrotten some time ago).

2013-06-28  Joseph Myers  <[hidden email]>

        * sysdeps/unix/sysv/linux/m68k/mmap64.c: New file.

diff --git a/ports/sysdeps/unix/sysv/linux/m68k/mmap64.c b/ports/sysdeps/unix/sysv/linux/m68k/mmap64.c
new file mode 100644
index 0000000..8bf8987
--- /dev/null
+++ b/ports/sysdeps/unix/sysv/linux/m68k/mmap64.c
@@ -0,0 +1,5 @@
+/* ColdFire and Sun 3 kernels have PAGE_SHIFT set to 13 and expect
+   mmap2 offset to be provided in 8K pages.  Determine the shift
+   dynamically with getpagesize.  */
+#define MMAP2_PAGE_SHIFT -1
+#include <sysdeps/unix/sysv/linux/mmap64.c>

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Miscellaneous ColdFire patches

Joseph Myers
In reply to this post by Andreas Schwab-2
On Fri, 28 Jun 2013, Andreas Schwab wrote:

> Please fix up the preprocessor indentation, possibly in a separate
> commit.

I have applied the patch, and this followup to fix the indentation.

diff --git a/ports/ChangeLog.m68k b/ports/ChangeLog.m68k
index 15c8da3..307665c 100644
--- a/ports/ChangeLog.m68k
+++ b/ports/ChangeLog.m68k
@@ -1,3 +1,7 @@
+2013-06-28  Joseph Myers  <[hidden email]>
+
+ * sysdeps/m68k/fpu_control.h: Fix preprocessor indentation.
+
 2013-06-28  Nathan Sidwell  <[hidden email]>
 
  * sysdeps/m68k/fpu_control.h [__mcoldfire__ && !__mcffpu__]
diff --git a/ports/sysdeps/m68k/fpu_control.h b/ports/sysdeps/m68k/fpu_control.h
index 848b5ba..7b22a95 100644
--- a/ports/sysdeps/m68k/fpu_control.h
+++ b/ports/sysdeps/m68k/fpu_control.h
@@ -55,58 +55,58 @@
 
 #if defined (__mcoldfire__) && !defined (__mcffpu__)
 
-#define _FPU_RESERVED 0xffffffff
-#define _FPU_DEFAULT  0x00000000
-#define _FPU_GETCW(cw) ((cw) = 0)
-#define _FPU_SETCW(cw) ((void) (cw))
+# define _FPU_RESERVED 0xffffffff
+# define _FPU_DEFAULT  0x00000000
+# define _FPU_GETCW(cw) ((cw) = 0)
+# define _FPU_SETCW(cw) ((void) (cw))
 
 #else
 
 /* masking of interrupts */
-#define _FPU_MASK_BSUN  0x8000
-#define _FPU_MASK_SNAN  0x4000
-#define _FPU_MASK_OPERR 0x2000
-#define _FPU_MASK_OVFL  0x1000
-#define _FPU_MASK_UNFL  0x0800
-#define _FPU_MASK_DZ    0x0400
-#define _FPU_MASK_INEX1 0x0200
-#define _FPU_MASK_INEX2 0x0100
+# define _FPU_MASK_BSUN  0x8000
+# define _FPU_MASK_SNAN  0x4000
+# define _FPU_MASK_OPERR 0x2000
+# define _FPU_MASK_OVFL  0x1000
+# define _FPU_MASK_UNFL  0x0800
+# define _FPU_MASK_DZ    0x0400
+# define _FPU_MASK_INEX1 0x0200
+# define _FPU_MASK_INEX2 0x0100
 
 /* precision control */
-#ifdef __mcoldfire__
-#define _FPU_DOUBLE   0x00
-#else
-#define _FPU_EXTENDED 0x00   /* RECOMMENDED */
-#define _FPU_DOUBLE   0x80
-#endif
-#define _FPU_SINGLE   0x40     /* DO NOT USE */
+# ifdef __mcoldfire__
+#  define _FPU_DOUBLE   0x00
+# else
+#  define _FPU_EXTENDED 0x00   /* RECOMMENDED */
+#  define _FPU_DOUBLE   0x80
+# endif
+# define _FPU_SINGLE   0x40     /* DO NOT USE */
 
 /* rounding control */
-#define _FPU_RC_NEAREST 0x00    /* RECOMMENDED */
-#define _FPU_RC_ZERO    0x10
-#define _FPU_RC_DOWN    0x20
-#define _FPU_RC_UP      0x30
+# define _FPU_RC_NEAREST 0x00    /* RECOMMENDED */
+# define _FPU_RC_ZERO    0x10
+# define _FPU_RC_DOWN    0x20
+# define _FPU_RC_UP      0x30
 
-#ifdef __mcoldfire__
-#define _FPU_RESERVED 0xFFFF800F
-#else
-#define _FPU_RESERVED 0xFFFF000F  /* Reserved bits in fpucr */
-#endif
+# ifdef __mcoldfire__
+#  define _FPU_RESERVED 0xFFFF800F
+# else
+#  define _FPU_RESERVED 0xFFFF000F  /* Reserved bits in fpucr */
+# endif
 
 
 /* Now two recommended fpucr */
 
 /* The fdlibm code requires no interrupts for exceptions.  Don't
    change the rounding mode, it would break long double I/O!  */
-#define _FPU_DEFAULT  0x00000000
+# define _FPU_DEFAULT  0x00000000
 
 /* IEEE:  same as above, but exceptions.  We must make it non-zero so
    that __setfpucw works.  This bit will be ignored.  */
-#define _FPU_IEEE     0x00000001
+# define _FPU_IEEE     0x00000001
 
 /* Macros for accessing the hardware control word.  */
-#define _FPU_GETCW(cw) __asm__ ("fmove%.l %!, %0" : "=dm" (cw))
-#define _FPU_SETCW(cw) __asm__ volatile ("fmove%.l %0, %!" : : "dm" (cw))
+# define _FPU_GETCW(cw) __asm__ ("fmove%.l %!, %0" : "=dm" (cw))
+# define _FPU_SETCW(cw) __asm__ volatile ("fmove%.l %0, %!" : : "dm" (cw))
 #endif
 
 /* Type of the control word.  */

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Miscellaneous ColdFire patches

Andreas Schwab-2
In reply to this post by Joseph Myers
"Joseph S. Myers" <[hidden email]> writes:

> * sysdeps/m68k/dl-trampoline.S (FMOVE): Define conditional on
> [__mcoldfire__] and [__mcffpu__].
> (FPSPACE): Likewise.
> (_dl_runtime_profile): Save and restore %fp0 with FMOVE, only if
> [FMOVE].  Use FPSPACE in stack offsets.

Ok.

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Ping for m68k MMAP2_PAGE_SHIFT fix

Joseph Myers
In reply to this post by Joseph Myers
All the pending fixes for sysdeps/unix/sysv/linux/mmap64.c's
MMAP2_PAGE_SHIFT == -1 case are now checked in.  Ping for this patch
<http://sourceware.org/ml/libc-ports/2013-06/msg00067.html> (untested)
that makes m68k use that case.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Ping for m68k MMAP2_PAGE_SHIFT fix

Andreas Schwab-2
"Joseph S. Myers" <[hidden email]> writes:

> All the pending fixes for sysdeps/unix/sysv/linux/mmap64.c's
> MMAP2_PAGE_SHIFT == -1 case are now checked in.  Ping for this patch
> <http://sourceware.org/ml/libc-ports/2013-06/msg00067.html> (untested)
> that makes m68k use that case.

../sysdeps/unix/sysv/linux/mmap64.c:46:7: warning: implicit declaration of function ‘__ffs’ [-Wimplicit-function-declaration]
       page_shift = __ffs (page_size) - 1;
       ^

(There is no need for the page_size variable, btw.)

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Ping Re: Ping for m68k MMAP2_PAGE_SHIFT fix

Joseph Myers
In reply to this post by Joseph Myers
On Tue, 20 Aug 2013, Joseph S. Myers wrote:

> All the pending fixes for sysdeps/unix/sysv/linux/mmap64.c's
> MMAP2_PAGE_SHIFT == -1 case are now checked in.  Ping for this patch
> <http://sourceware.org/ml/libc-ports/2013-06/msg00067.html> (untested)
> that makes m68k use that case.

Ping again....

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Miscellaneous ColdFire patches

Andreas Schwab-2
In reply to this post by Joseph Myers
"Joseph S. Myers" <[hidden email]> writes:

> 2013-06-28  Joseph Myers  <[hidden email]>
>
> * sysdeps/unix/sysv/linux/m68k/mmap64.c: New file.

Ok.

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."