diff mbox

Add _STRING_INLINE_unaligned

Message ID CAMe9rOrGx5ymaN7o=qSsaQ+hDy7tO9W48itu1C4iv3paEoMtCw@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu Jan. 15, 2016, 5:13 p.m. UTC 
  On Thu, Jan 14, 2016 at 11:24 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jan 8, 2016 at 9:37 AM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>> As discussed in https://sourceware.org/ml/libc-alpha/2015-10/msg00403.html ,
>> the setting of _STRING_ARCH_unaligned currently controls the external
>> GLIBC ABI as well as selecting the use of unaligned accesses withing GLIBC.
>>
>> Since _STRING_ARCH_unaligned was recently changed for AArch64, this
>> would potentially break the ABI in GLIBC 2.23, so split the uses and add
>> _STRING_INLINE_unaligned to select the string ABI. This setting must be fixed for
>> each target, while _STRING_ARCH_unaligned may be changed from release to release.
>>
>> Built and tested on AArch64, x86 and x64. OK for GLIBC 2.23?
>>
>> ChangeLog:
>> 2016-01-08  Wilco Dijkstra  <wdijkstr@arm.com>
>>
>>         * bits/string.h (_STRING_INLINE_unaligned): New define.
>>         * string/string-inlines.c: Use _STRING_INLINE_unaligned.
>>         * string/bits/string2.h (_STRING_ARCH_unaligned): Remove
>>         unused conditional includes.
>>         * sysdeps/aarch64/bits/string.h (_STRING_INLINE_unaligned): Add define.
>>         * sysdeps/m68k/m680x0/m68020/bits/string.h
>>         (_STRING_INLINE_unaligned): Likewise.
>>         * sysdeps/sparc/bits/string.h (_STRING_INLINE_unaligned): Likewise.
>>         * sysdeps/s390/bits/string.h (_STRING_INLINE_unaligned): Likewise.
>>         * sysdeps/x86/bits/string.h (_STRING_INLINE_unaligned): Likewise.
>>
>>
>>
>
> This patch can't be applied since it has odd characters.
>
> Here is a patch to support -Os build.  Tested on x86-64 and
> i686 without any code changes.
>
> Can you combine your patch with mine to move
> _STRING_ARCH_unaligned to string_private.h and add
> STRING_INLINE_unaligned to bits/string.h?
>

Here is the combined patch.  _STRING_ARCH_unaligned is
removed from all installed header files:

[hjl@gnu-6 glibc]$ git grep _STRING_ARCH_unaligned | grep -v ChangeLog
bits/string.h:   after the first release of a new target (unlike
_STRING_ARCH_unaligned
crypt/md5.c:#if !_STRING_ARCH_unaligned
crypt/sha256.c:#if _STRING_ARCH_unaligned
crypt/sha256.c:#if !_STRING_ARCH_unaligned
crypt/sha512.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/gconv_simple.c:#if !_STRING_ARCH_unaligned
iconv/loop.c:#if _STRING_ARCH_unaligned || !defined DEFINE_UNALIGNED
iconv/loop.c:#if !defined DEFINE_UNALIGNED && !_STRING_ARCH_unaligned \
iconv/skeleton.c:#if _STRING_ARCH_unaligned
iconv/skeleton.c:  (!_STRING_ARCH_unaligned              \
include/arpa/nameser.h:#if _STRING_ARCH_unaligned
include/string.h:/* Get _STRING_ARCH_unaligned.  */
nscd/nscd_gethst_r.c:#if !_STRING_ARCH_unaligned
nscd/nscd_getserv_r.c:#if !_STRING_ARCH_unaligned
nscd/nscd_helper.c:#if !_STRING_ARCH_unaligned
nscd/nscd_helper.c:#if !_STRING_ARCH_unaligned
nscd/nscd_helper.c:#if !_STRING_ARCH_unaligned
resolv/res_send.c:#if _STRING_ARCH_unaligned
resolv/res_send.c:#if _STRING_ARCH_unaligned
stdlib/getenv.c:#if __BYTE_ORDER == __LITTLE_ENDIAN || !_STRING_ARCH_unaligned
stdlib/getenv.c:#if _STRING_ARCH_unaligned
stdlib/getenv.c:#if _STRING_ARCH_unaligned
stdlib/getenv.c:#if _STRING_ARCH_unaligned
sysdeps/aarch64/string_private.h:/* Define _STRING_ARCH_unaligned.
AArch64 version.
sysdeps/aarch64/string_private.h:#define _STRING_ARCH_unaligned 1
sysdeps/generic/string_private.h:/* Define _STRING_ARCH_unaligned.
Generic version.
sysdeps/generic/string_private.h:#define _STRING_ARCH_unaligned   0
sysdeps/m68k/m680x0/m68020/string_private.h:/* Define
_STRING_ARCH_unaligned.  m680x0 version, x >= 2.
sysdeps/m68k/m680x0/m68020/string_private.h:#define _STRING_ARCH_unaligned   1
sysdeps/s390/string_private.h:/* Define _STRING_ARCH_unaligned.  S/390 version.
sysdeps/s390/string_private.h:#define _STRING_ARCH_unaligned   1
sysdeps/x86/string_private.h:/* Define _STRING_ARCH_unaligned.
i486/x86-64 version.
sysdeps/x86/string_private.h:#define _STRING_ARCH_unaligned   1
[hjl@gnu-6 glibc]$

Tested on x86-64, x32 and i686.  OK for trunk?

Comments

Wilco Dijkstra Jan. 20, 2016, 2:57 p.m. UTC | #1 
H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > Here is the combined patch.  _STRING_ARCH_unaligned is
> > removed from all installed header files:

This looks fine to me, it's smaller than my patch as it doesn't depend on any previous patches.
So it seems best to go with your patch assuming the maintainers agree to this for 2.23.

Wilco
diff mbox

Patch

From 0d32fcb814a7ea7e857a967e96e5c76205bd729b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 15 Jan 2016 08:11:23 -0800
Subject: [PATCH] Add _STRING_INLINE_unaligned and string_private.h

As discussed in

https://sourceware.org/ml/libc-alpha/2015-10/msg00403.html

the setting of _STRING_ARCH_unaligned currently controls the external
GLIBC ABI as well as selecting the use of unaligned accesses withing
GLIBC.

Since _STRING_ARCH_unaligned was recently changed for AArch64, this
would potentially break the ABI in GLIBC 2.23, so split the uses and add
_STRING_INLINE_unaligned to select the string ABI. This setting must be
fixed for each target, while _STRING_ARCH_unaligned may be changed from
release to release.  _STRING_ARCH_unaligned is used unconditionally in
glibc.  But <bits/string.h>, which defines _STRING_ARCH_unaligned, isn't
included with -Os.  Since _STRING_ARCH_unaligned is internal to glibc and
may change between glibc releases, it should be made private to glibc.
_STRING_ARCH_unaligned should defined in the new string_private.h heade
file which is included unconditionally from internal <string.h> for glibc
build.

2016-01-15  H.J. Lu  <hongjiu.lu@intel.com>
	    Wilco Dijkstra  <wdijkstr@arm.com>

	[BZ #19462]
	* bits/string.h (_STRING_ARCH_unaligned): Renamed to ...
	(_STRING_INLINE_unaligned): This.
	* include/string.h: Include <string_private.h>.
	* string/bits/string2.h: Replace _STRING_ARCH_unaligned with
	_STRING_INLINE_unaligned.
	* sysdeps/aarch64/bits/string.h (_STRING_ARCH_unaligned): Removed.
	(_STRING_INLINE_unaligned): New.
	* sysdeps/aarch64/string_private.h: New file.
	* sysdeps/generic/string_private.h: Likewise.
	* sysdeps/m68k/m680x0/m68020/string_private.h: Likewise.
	* sysdeps/s390/string_private.h: Likewise.
	* sysdeps/x86/string_private.h: Likewise.
	* sysdeps/m68k/m680x0/m68020/bits/string.h
	(_STRING_ARCH_unaligned): Renamed to ...
	(_STRING_INLINE_unaligned): This.
	* sysdeps/s390/bits/string.h (_STRING_ARCH_unaligned): Renamed
	to ...
	(_STRING_INLINE_unaligned): This.
	* sysdeps/sparc/bits/string.h (_STRING_ARCH_unaligned): Renamed
	to ...
	(_STRING_INLINE_unaligned): This.
	* sysdeps/x86/bits/string.h (_STRING_ARCH_unaligned): Renamed
	to ...
	(_STRING_INLINE_unaligned): This.
---
 bits/string.h                               |  8 ++++++--
 include/string.h                            |  3 +++
 string/bits/string2.h                       | 10 +++++-----
 sysdeps/aarch64/bits/string.h               |  4 ++--
 sysdeps/aarch64/string_private.h            | 20 ++++++++++++++++++++
 sysdeps/generic/string_private.h            | 21 +++++++++++++++++++++
 sysdeps/m68k/m680x0/m68020/bits/string.h    |  5 ++---
 sysdeps/m68k/m680x0/m68020/string_private.h | 21 +++++++++++++++++++++
 sysdeps/s390/bits/string.h                  |  4 ++--
 sysdeps/s390/string_private.h               | 20 ++++++++++++++++++++
 sysdeps/sparc/bits/string.h                 |  4 ++--
 sysdeps/x86/bits/string.h                   |  4 ++--
 sysdeps/x86/string_private.h                | 20 ++++++++++++++++++++
 13 files changed, 126 insertions(+), 18 deletions(-)
 create mode 100644 sysdeps/aarch64/string_private.h
 create mode 100644 sysdeps/generic/string_private.h
 create mode 100644 sysdeps/m68k/m680x0/m68020/string_private.h
 create mode 100644 sysdeps/s390/string_private.h
 create mode 100644 sysdeps/x86/string_private.h

diff --git a/bits/string.h b/bits/string.h
index b88a6bc..89c627c 100644
--- a/bits/string.h
+++ b/bits/string.h
@@ -8,7 +8,11 @@ 
 #ifndef _BITS_STRING_H
 #define _BITS_STRING_H	1
 
-/* Define if architecture can access unaligned multi-byte variables.  */
-#define _STRING_ARCH_unaligned   0
+/* Define whether to use the unaligned string inline ABI.
+   The string inline functions are an external ABI, thus cannot be changed
+   after the first release of a new target (unlike _STRING_ARCH_unaligned
+   which may be changed from release to release).  Targets must support
+   unaligned accesses in hardware if either define is set to true.  */
+#define _STRING_INLINE_unaligned   0
 
 #endif /* bits/string.h */
diff --git a/include/string.h b/include/string.h
index a684fd9..e145bfd 100644
--- a/include/string.h
+++ b/include/string.h
@@ -46,6 +46,9 @@  extern int __ffs (int __i) __attribute__ ((const));
 extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen);
 #endif
 
+/* Get _STRING_ARCH_unaligned.  */
+#include <string_private.h>
+
 /* Now the real definitions.  We do this here since some of the functions
    above are defined as macros in the headers.  */
 #include <string/string.h>
diff --git a/string/bits/string2.h b/string/bits/string2.h
index b93be06..8200ef1 100644
--- a/string/bits/string2.h
+++ b/string/bits/string2.h
@@ -46,7 +46,7 @@ 
 # endif
 #endif
 
-#if _STRING_ARCH_unaligned
+#if _STRING_INLINE_unaligned
 /* If we can do unaligned memory accesses we must know the endianess.  */
 # include <endian.h>
 # include <bits/types.h>
@@ -95,7 +95,7 @@  __STRING2_COPY_TYPE (8);
 /* Set N bytes of S to C.  */
 #if !defined _HAVE_STRING_ARCH_memset
 # if !__GNUC_PREREQ (3, 0)
-#  if _STRING_ARCH_unaligned
+#  if _STRING_INLINE_unaligned
 #   define memset(s, c, n) \
   (__extension__ (__builtin_constant_p (n) && (n) <= 16			      \
 		  ? ((n) == 1						      \
@@ -223,7 +223,7 @@  __STRING2_COPY_TYPE (8);
 #  endif
 
 #  if !__GNUC_PREREQ (3, 0) || defined _FORCE_INLINES
-#   if _STRING_ARCH_unaligned
+#   if _STRING_INLINE_unaligned
 #    ifndef _FORCE_INLINES
 #     define __mempcpy_args(src) \
      ((const char *) (src))[0], ((const char *) (src))[2],		      \
@@ -419,7 +419,7 @@  extern void *__rawmemchr (const void *__s, int __c);
 		  : strcpy (dest, src)))
 # endif
 
-# if _STRING_ARCH_unaligned
+# if _STRING_INLINE_unaligned
 #  ifndef _FORCE_INLINES
 #   define __strcpy_args(src) \
      __extension__ __STRING2_SMALL_GET16 (src, 0),			      \
@@ -598,7 +598,7 @@  __strcpy_small (char *__dest,
 #  endif
 
 #  if !__GNUC_PREREQ (3, 0) || defined _FORCE_INLINES
-#   if _STRING_ARCH_unaligned
+#   if _STRING_INLINE_unaligned
 #    ifndef _FORCE_INLINES
 #     define __stpcpy_args(src) \
      __extension__ __STRING2_SMALL_GET16 (src, 0),			      \
diff --git a/sysdeps/aarch64/bits/string.h b/sysdeps/aarch64/bits/string.h
index 3c2a50b..0a508f7 100644
--- a/sysdeps/aarch64/bits/string.h
+++ b/sysdeps/aarch64/bits/string.h
@@ -20,5 +20,5 @@ 
 # error "Never use <bits/string.h> directly; include <string.h> instead."
 #endif
 
-/* AArch64 implementations support efficient unaligned access.  */
-#define _STRING_ARCH_unaligned 1
+/* AArch64 uses the aligned string inline ABI.  */
+#define _STRING_INLINE_unaligned 0
diff --git a/sysdeps/aarch64/string_private.h b/sysdeps/aarch64/string_private.h
new file mode 100644
index 0000000..8b194a4
--- /dev/null
+++ b/sysdeps/aarch64/string_private.h
@@ -0,0 +1,20 @@ 
+/* Define _STRING_ARCH_unaligned.  AArch64 version.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* AArch64 implementations support efficient unaligned access.  */
+#define _STRING_ARCH_unaligned 1
diff --git a/sysdeps/generic/string_private.h b/sysdeps/generic/string_private.h
new file mode 100644
index 0000000..c880aae
--- /dev/null
+++ b/sysdeps/generic/string_private.h
@@ -0,0 +1,21 @@ 
+/* Define _STRING_ARCH_unaligned.  Generic version.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Define to 1 if architecture can access unaligned multi-byte
+   variables.  */
+#define _STRING_ARCH_unaligned   0
diff --git a/sysdeps/m68k/m680x0/m68020/bits/string.h b/sysdeps/m68k/m680x0/m68020/bits/string.h
index 88505bb..78c9594 100644
--- a/sysdeps/m68k/m680x0/m68020/bits/string.h
+++ b/sysdeps/m68k/m680x0/m68020/bits/string.h
@@ -20,6 +20,5 @@ 
 # error "Never use <bits/string.h> directly; include <string.h> instead."
 #endif
 
-/* Currently the only purpose of this file is to tell the generic inline
-   macros that unaligned memory access is possible.  */
-#define _STRING_ARCH_unaligned	1
+/* Use the unaligned string inline ABI.  */
+#define _STRING_INLINE_unaligned 1
diff --git a/sysdeps/m68k/m680x0/m68020/string_private.h b/sysdeps/m68k/m680x0/m68020/string_private.h
new file mode 100644
index 0000000..6846663
--- /dev/null
+++ b/sysdeps/m68k/m680x0/m68020/string_private.h
@@ -0,0 +1,21 @@ 
+/* Define _STRING_ARCH_unaligned.  m680x0 version, x >= 2.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Tell the generic inline macros that unaligned memory access is
+   possible.  */
+#define _STRING_ARCH_unaligned   1
diff --git a/sysdeps/s390/bits/string.h b/sysdeps/s390/bits/string.h
index 09201d9..39e0b7f 100644
--- a/sysdeps/s390/bits/string.h
+++ b/sysdeps/s390/bits/string.h
@@ -21,8 +21,8 @@ 
 # error "Never use <bits/string.h> directly; include <string.h> instead."
 #endif
 
-/* The s390 processors can access unaligned multi-byte variables.  */
-#define _STRING_ARCH_unaligned	1
+/* Use the unaligned string inline ABI.  */
+#define _STRING_INLINE_unaligned 1
 
 /* We only provide optimizations if the user selects them and if
    GNU CC is used.  */
diff --git a/sysdeps/s390/string_private.h b/sysdeps/s390/string_private.h
new file mode 100644
index 0000000..9e11eee
--- /dev/null
+++ b/sysdeps/s390/string_private.h
@@ -0,0 +1,20 @@ 
+/* Define _STRING_ARCH_unaligned.  S/390 version.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* The s390 processors can access unaligned multi-byte variables.  */
+#define _STRING_ARCH_unaligned   1
diff --git a/sysdeps/sparc/bits/string.h b/sysdeps/sparc/bits/string.h
index 3054f62..10beca6 100644
--- a/sysdeps/sparc/bits/string.h
+++ b/sysdeps/sparc/bits/string.h
@@ -20,8 +20,8 @@ 
 # error "Never use <bits/string.h> directly; include <string.h> instead."
 #endif
 
-/* Define if architecture can access unaligned multi-byte variables.  */
-#define _STRING_ARCH_unaligned   0
+/* sparc uses the aligned string inline ABI.  */
+#define _STRING_INLINE_unaligned 0
 
 /* sparc32 and sparc64 strchr(x, '\0') perform better than
    __rawmemchr(x, '\0').  */
diff --git a/sysdeps/x86/bits/string.h b/sysdeps/x86/bits/string.h
index 1a0682b..e4e019f 100644
--- a/sysdeps/x86/bits/string.h
+++ b/sysdeps/x86/bits/string.h
@@ -20,8 +20,8 @@ 
 # error "Never use <bits/string.h> directly; include <string.h> instead."
 #endif
 
-/* The ix86 processors can access unaligned multi-byte variables.  */
-#define _STRING_ARCH_unaligned	1
+/* Use the unaligned string inline ABI.  */
+#define _STRING_INLINE_unaligned 1
 
 /* Enable inline functions only for i486 or better when compiling for
    ia32.  */
diff --git a/sysdeps/x86/string_private.h b/sysdeps/x86/string_private.h
new file mode 100644
index 0000000..e7281eb
--- /dev/null
+++ b/sysdeps/x86/string_private.h
@@ -0,0 +1,20 @@ 
+/* Define _STRING_ARCH_unaligned.  i486/x86-64 version.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* The ix86 processors can access unaligned multi-byte variables.  */
+#define _STRING_ARCH_unaligned   1
-- 
2.5.0