This is the mail archive of the
libc-ports@sources.redhat.com
mailing list for the libc-ports project.
Re: [PATCH 1a/9] Tilera (and Linux asm-generic) support for glibc
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Chris Metcalf <cmetcalf at tilera dot com>
- Cc: libc-ports at sourceware dot org, Arnd Bergmann <arnd at arndb dot de>, Linas Vepstas <linas at codeaurora dot org>, Guan Xuetao <gxt at mprc dot pku dot edu dot cn>, Jonas Bonn <jonas at southpole dot se>, Chen Liqin <liqin dot chen at gmail dot com>
- Date: Fri, 4 Nov 2011 21:45:34 +0000 (UTC)
- Subject: Re: [PATCH 1a/9] Tilera (and Linux asm-generic) support for glibc
- References: <201111031955.pA3Jt3sC024069@farm-0002.internal.tilera.com> <201111041214.pA4CEInn024910@farm-0002.internal.tilera.com>
On Fri, 4 Nov 2011, Chris Metcalf wrote:
> diff --git a/sysdeps/tile/Versions b/sysdeps/tile/Versions
> new file mode 100644
> index 0000000..b275d7f
> --- /dev/null
> +++ b/sysdeps/tile/Versions
> @@ -0,0 +1,6 @@
> +libc {
> + GLIBC_2.12 {
> + # name requested by gcc community.
> + __mcount;
> + }
GLIBC_2.12 does not make sense for a new symbol being introduced in 2.15;
GLIBC_2.15 would be appropriate.
How important is binary compatibility with existing binaries using shared
libraries? Because a new port should generally have a shlib-versions file
with a DEFAULT line setting the minimum symbol version - thus the natural
thing to do would be to set that to GLIBC_2.15 and exclude all
compatibility for earlier versions than that. But I don't see such a file
here at all (even for an earlier version from which the port was started),
although I doubt that there really ever existed glibc 2.0 (for example)
for these targets.
> +#define atomic_full_barrier() __insn_mf ()
It seems unfortunate for a new port to define atomics in terms of
target-specific intrinsics - or, worse, asms. I'm pretty sure the new
__atomic_* built-in functions in the cxx-mem-model code now being merged
for GCC 4.7 can represent all the variants used in glibc. So implement
the new patterns for your GCC ports which should also be merged for 4.7
and you could have the glibc ports just use essentially generic
definitions in terms of __atomic_*.
> diff --git a/sysdeps/tile/bits/byteswap.h b/sysdeps/tile/bits/byteswap.h
There's nothing *wrong* with this file; it just seems:
* probably unnecessary (current GCC can detect bswap patterns that should
include those used by the generic bits/byteswap.h);
* generic (it would be better to make libc's bits/byteswap.h use the
built-ins if __GNUC_PREREQ (4, 3).
> +/* Test for negative number. Used in the signbit() macro. */
> +__MATH_INLINE int
> +__NTH (__signbitf (float __x))
> +{
> + __extension__ union { float __f; int __i; } __u = { __f: __x };
> + return __u.__i < 0;
> +}
> +__MATH_INLINE int
> +__NTH (__signbit (double __x))
> +{
> + __extension__ union { double __d; long long __i; } __u = { __d: __x };
> + return __u.__i < 0;
> +}
Using __builtin_signbit / __builtin_signbitf would seem better than code
using unions (though it ought to expand the same). (If GCC PR 36757 were
fixed then changing the definition of the signbit macro just to call
__builtin_signbit for newer GCC would be the right approach, but that's a
separate matter.)
> +#ifndef __tilegx__
> + /* Updating a PLT entry atomically on TILEPro is very tricky, since
> + bundles are 8 bytes, we can only atomically write four bytes at a
> + time, and jump instructions span both words of the bundle.
> +
> + We carefully update one word and then the other in an order
> + such that the intermediate values seen by any reader are
> + all valid code sequences. It is fortuitous that such a sequence
> + exists.
What, your ABI is using writable-and-executable PLTs? That's a rather
unfortunate choice for a new ABI (and I didn't see a patch to
check-textrel.c in the libc changes to allow it, not that I'm sure such a
change would actually be accepted for a new architecture); it's generally
considered a bad idea to have any segments that are both writable and
executable.
> diff --git a/sysdeps/tile/preconfigure b/sysdeps/tile/preconfigure
> new file mode 100644
> index 0000000..61e1f96
> --- /dev/null
> +++ b/sysdeps/tile/preconfigure
> @@ -0,0 +1,14 @@
> +# This is a -*- sh -*-
> +case "$machine" in
> + tilepro)
> + base_machine=tile machine=tile/tilepro ;;
> + tilegx)
> + base_machine=tile machine=tile/tilegx/tilegx64 ;;
> + tilegx32)
> + base_machine=tile machine=tile/tilegx/tilegx32 ;;
I don't see any sign of distinct tilegx/tilegx32 target triplets in the
GCC port (as in, I don't see any sign of code to change the default based
on the target triplet). config.guess appears to go purely on what uname
reports. It's not wrong to condition on the target triplet here, but it
might be friendlier to users to check how the compiler ($CC $CFLAGS
$CPPFLAGS) is actually configured.
> +/* Since C identifiers are not normally prefixed with an underscore
> + on this system, the asm identifier `syscall_error' intrudes on the
> + C name space. Make sure we use an innocuous name. */
> +#define syscall_error __syscall_error
> +#define mcount _mcount
__mcount?
--
Joseph S. Myers
joseph@codesourcery.com