* [PATCH 1/7] crypto: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-15 17:55 ` Wilco Dijkstra
2023-02-13 13:55 ` [PATCH 2/7] stdlib: Simplify getenv Adhemerval Zanella
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
Assume unaligned inputs on all cases. The code is built and used only
in compat mode.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
crypt/md5.c | 24 ++++--------------------
crypt/sha256.c | 28 ++++------------------------
crypt/sha512.c | 26 +++++---------------------
3 files changed, 13 insertions(+), 65 deletions(-)
diff --git a/crypt/md5.c b/crypt/md5.c
index c7a232ad38..03240a9a36 100644
--- a/crypt/md5.c
+++ b/crypt/md5.c
@@ -229,27 +229,11 @@ md5_process_bytes (const void *buffer, size_t len, struct md5_ctx *ctx)
/* Process available complete blocks. */
if (len >= 64)
{
-#if !_STRING_ARCH_unaligned
-/* To check alignment gcc has an appropriate operator. Other
- compilers don't. */
-# if __GNUC__ >= 2
-# define UNALIGNED_P(p) (((md5_uintptr) p) % __alignof__ (md5_uint32) != 0)
-# else
-# define UNALIGNED_P(p) (((md5_uintptr) p) % sizeof (md5_uint32) != 0)
-# endif
- if (UNALIGNED_P (buffer))
- while (len > 64)
- {
- __md5_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
- buffer = (const char *) buffer + 64;
- len -= 64;
- }
- else
-#endif
+ while (len > 64)
{
- __md5_process_block (buffer, len & ~63, ctx);
- buffer = (const char *) buffer + (len & ~63);
- len &= 63;
+ __md5_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
+ buffer = (const char *) buffer + 64;
+ len -= 64;
}
}
diff --git a/crypt/sha256.c b/crypt/sha256.c
index 93b73997c7..96153d67dc 100644
--- a/crypt/sha256.c
+++ b/crypt/sha256.c
@@ -120,13 +120,9 @@ __sha256_finish_ctx (struct sha256_ctx *ctx, void *resbuf)
memcpy (&ctx->buffer[bytes], fillbuf, pad);
/* Put the 64-bit file length in *bits* at the end of the buffer. */
-#if _STRING_ARCH_unaligned
- ctx->buffer64[(bytes + pad) / 8] = SWAP64 (ctx->total64 << 3);
-#else
ctx->buffer32[(bytes + pad + 4) / 4] = SWAP (ctx->total[TOTAL64_low] << 3);
ctx->buffer32[(bytes + pad) / 4] = SWAP ((ctx->total[TOTAL64_high] << 3)
| (ctx->total[TOTAL64_low] >> 29));
-#endif
/* Process last bytes. */
__sha256_process_block (ctx->buffer, bytes + pad + 8, ctx);
@@ -169,27 +165,11 @@ __sha256_process_bytes (const void *buffer, size_t len, struct sha256_ctx *ctx)
/* Process available complete blocks. */
if (len >= 64)
{
-#if !_STRING_ARCH_unaligned
-/* To check alignment gcc has an appropriate operator. Other
- compilers don't. */
-# if __GNUC__ >= 2
-# define UNALIGNED_P(p) (((uintptr_t) p) % __alignof__ (uint32_t) != 0)
-# else
-# define UNALIGNED_P(p) (((uintptr_t) p) % sizeof (uint32_t) != 0)
-# endif
- if (UNALIGNED_P (buffer))
- while (len > 64)
- {
- __sha256_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
- buffer = (const char *) buffer + 64;
- len -= 64;
- }
- else
-#endif
+ while (len > 64)
{
- __sha256_process_block (buffer, len & ~63, ctx);
- buffer = (const char *) buffer + (len & ~63);
- len &= 63;
+ __sha256_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
+ buffer = (const char *) buffer + 64;
+ len -= 64;
}
}
diff --git a/crypt/sha512.c b/crypt/sha512.c
index d7e51b3604..ceabad1bf7 100644
--- a/crypt/sha512.c
+++ b/crypt/sha512.c
@@ -192,28 +192,12 @@ __sha512_process_bytes (const void *buffer, size_t len, struct sha512_ctx *ctx)
/* Process available complete blocks. */
if (len >= 128)
{
-#if !_STRING_ARCH_unaligned
-/* To check alignment gcc has an appropriate operator. Other
- compilers don't. */
-# if __GNUC__ >= 2
-# define UNALIGNED_P(p) (((uintptr_t) p) % __alignof__ (uint64_t) != 0)
-# else
-# define UNALIGNED_P(p) (((uintptr_t) p) % sizeof (uint64_t) != 0)
-# endif
- if (UNALIGNED_P (buffer))
- while (len > 128)
- {
- __sha512_process_block (memcpy (ctx->buffer, buffer, 128), 128,
- ctx);
- buffer = (const char *) buffer + 128;
- len -= 128;
- }
- else
-#endif
+ while (len > 128)
{
- __sha512_process_block (buffer, len & ~127, ctx);
- buffer = (const char *) buffer + (len & ~127);
- len &= 127;
+ __sha512_process_block (memcpy (ctx->buffer, buffer, 128), 128,
+ ctx);
+ buffer = (const char *) buffer + 128;
+ len -= 128;
}
}
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 1/7] crypto: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 ` [PATCH 1/7] crypto: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
@ 2023-02-15 17:55 ` Wilco Dijkstra
0 siblings, 0 replies; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 17:55 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
This looks good to me.
Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
diff --git a/crypt/md5.c b/crypt/md5.c
index c7a232ad38..03240a9a36 100644
--- a/crypt/md5.c
+++ b/crypt/md5.c
@@ -229,27 +229,11 @@ md5_process_bytes (const void *buffer, size_t len, struct md5_ctx *ctx)
/* Process available complete blocks. */
if (len >= 64)
{
-#if !_STRING_ARCH_unaligned
-/* To check alignment gcc has an appropriate operator. Other
- compilers don't. */
-# if __GNUC__ >= 2
-# define UNALIGNED_P(p) (((md5_uintptr) p) % __alignof__ (md5_uint32) != 0)
-# else
-# define UNALIGNED_P(p) (((md5_uintptr) p) % sizeof (md5_uint32) != 0)
-# endif
- if (UNALIGNED_P (buffer))
- while (len > 64)
- {
- __md5_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
- buffer = (const char *) buffer + 64;
- len -= 64;
- }
- else
-#endif
+ while (len > 64)
{
- __md5_process_block (buffer, len & ~63, ctx);
- buffer = (const char *) buffer + (len & ~63);
- len &= 63;
+ __md5_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
+ buffer = (const char *) buffer + 64;
+ len -= 64;
}
}
OK
diff --git a/crypt/sha256.c b/crypt/sha256.c
index 93b73997c7..96153d67dc 100644
--- a/crypt/sha256.c
+++ b/crypt/sha256.c
@@ -120,13 +120,9 @@ __sha256_finish_ctx (struct sha256_ctx *ctx, void *resbuf)
memcpy (&ctx->buffer[bytes], fillbuf, pad);
/* Put the 64-bit file length in *bits* at the end of the buffer. */
-#if _STRING_ARCH_unaligned
- ctx->buffer64[(bytes + pad) / 8] = SWAP64 (ctx->total64 << 3);
-#else
ctx->buffer32[(bytes + pad + 4) / 4] = SWAP (ctx->total[TOTAL64_low] << 3);
ctx->buffer32[(bytes + pad) / 4] = SWAP ((ctx->total[TOTAL64_high] << 3)
| (ctx->total[TOTAL64_low] >> 29));
-#endif
/* Process last bytes. */
__sha256_process_block (ctx->buffer, bytes + pad + 8, ctx);
@@ -169,27 +165,11 @@ __sha256_process_bytes (const void *buffer, size_t len, struct sha256_ctx *ctx)
/* Process available complete blocks. */
if (len >= 64)
{
-#if !_STRING_ARCH_unaligned
-/* To check alignment gcc has an appropriate operator. Other
- compilers don't. */
-# if __GNUC__ >= 2
-# define UNALIGNED_P(p) (((uintptr_t) p) % __alignof__ (uint32_t) != 0)
-# else
-# define UNALIGNED_P(p) (((uintptr_t) p) % sizeof (uint32_t) != 0)
-# endif
- if (UNALIGNED_P (buffer))
- while (len > 64)
- {
- __sha256_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
- buffer = (const char *) buffer + 64;
- len -= 64;
- }
- else
-#endif
+ while (len > 64)
{
- __sha256_process_block (buffer, len & ~63, ctx);
- buffer = (const char *) buffer + (len & ~63);
- len &= 63;
+ __sha256_process_block (memcpy (ctx->buffer, buffer, 64), 64, ctx);
+ buffer = (const char *) buffer + 64;
+ len -= 64;
}
}
OK
diff --git a/crypt/sha512.c b/crypt/sha512.c
index d7e51b3604..ceabad1bf7 100644
--- a/crypt/sha512.c
+++ b/crypt/sha512.c
@@ -192,28 +192,12 @@ __sha512_process_bytes (const void *buffer, size_t len, struct sha512_ctx *ctx)
/* Process available complete blocks. */
if (len >= 128)
{
-#if !_STRING_ARCH_unaligned
-/* To check alignment gcc has an appropriate operator. Other
- compilers don't. */
-# if __GNUC__ >= 2
-# define UNALIGNED_P(p) (((uintptr_t) p) % __alignof__ (uint64_t) != 0)
-# else
-# define UNALIGNED_P(p) (((uintptr_t) p) % sizeof (uint64_t) != 0)
-# endif
- if (UNALIGNED_P (buffer))
- while (len > 128)
- {
- __sha512_process_block (memcpy (ctx->buffer, buffer, 128), 128,
- ctx);
- buffer = (const char *) buffer + 128;
- len -= 128;
- }
- else
-#endif
+ while (len > 128)
{
- __sha512_process_block (buffer, len & ~127, ctx);
- buffer = (const char *) buffer + (len & ~127);
- len &= 127;
+ __sha512_process_block (memcpy (ctx->buffer, buffer, 128), 128,
+ ctx);
+ buffer = (const char *) buffer + 128;
+ len -= 128;
}
}
OK
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/7] stdlib: Simplify getenv
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
2023-02-13 13:55 ` [PATCH 1/7] crypto: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-15 17:50 ` Wilco Dijkstra
2023-02-13 13:55 ` [PATCH 3/7] nscd: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
And remove _STRING_ARCH_unaligned usage. The small size optimization
just adds boilerplate code.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
stdlib/getenv.c | 63 ++++---------------------------------------------
1 file changed, 4 insertions(+), 59 deletions(-)
diff --git a/stdlib/getenv.c b/stdlib/getenv.c
index e3157ce2f3..449bba1e16 100644
--- a/stdlib/getenv.c
+++ b/stdlib/getenv.c
@@ -15,76 +15,21 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <endian.h>
-#include <errno.h>
-#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-
-/* Return the value of the environment variable NAME. This implementation
- is tuned a bit in that it assumes no environment variable has an empty
- name which of course should always be true. We have a special case for
- one character names so that for the general case we can assume at least
- two characters which we can access. By doing this we can avoid using the
- `strncmp' most of the time. */
char *
getenv (const char *name)
{
- char **ep;
- uint16_t name_start;
-
if (__environ == NULL || name[0] == '\0')
return NULL;
- if (name[1] == '\0')
- {
- /* The name of the variable consists of only one character. Therefore
- the first two characters of the environment entry are this character
- and a '=' character. */
-#if __BYTE_ORDER == __LITTLE_ENDIAN || !_STRING_ARCH_unaligned
- name_start = ('=' << 8) | *(const unsigned char *) name;
-#else
- name_start = '=' | ((*(const unsigned char *) name) << 8);
-#endif
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
- if (name_start == ep_start)
- return &(*ep)[2];
- }
- }
- else
+ size_t len = __strchrnul (name, '=') - name;
+ for (char **ep = __environ; *ep != NULL; ++ep)
{
- size_t len = strlen (name);
-#if _STRING_ARCH_unaligned
- name_start = *(const uint16_t *) name;
-#else
- name_start = (((const unsigned char *) name)[0]
- | (((const unsigned char *) name)[1] << 8));
-#endif
- len -= 2;
- name += 2;
-
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
-
- if (name_start == ep_start && !strncmp (*ep + 2, name, len)
- && (*ep)[len + 2] == '=')
- return &(*ep)[len + 3];
- }
+ if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
+ return *ep + len + 1;
}
return NULL;
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-13 13:55 ` [PATCH 2/7] stdlib: Simplify getenv Adhemerval Zanella
@ 2023-02-15 17:50 ` Wilco Dijkstra
2023-02-16 13:49 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 17:50 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
+ size_t len = __strchrnul (name, '=') - name;
This is a change in behaviour, ignoring anything behind a '=' in name. What is
wrong with strlen?
+ for (char **ep = __environ; *ep != NULL; ++ep)
{
- size_t len = strlen (name);
-#if _STRING_ARCH_unaligned
- name_start = *(const uint16_t *) name;
-#else
- name_start = (((const unsigned char *) name)[0]
- | (((const unsigned char *) name)[1] << 8));
-#endif
- len -= 2;
- name += 2;
-
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
-
- if (name_start == ep_start && !strncmp (*ep + 2, name, len)
- && (*ep)[len + 2] == '=')
- return &(*ep)[len + 3];
- }
+ if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
+ return *ep + len + 1;
It's still useful to keep a check for the first character, eg. name[0] == (*ep)[0] as this
is about twice as fast as always calling strncmp.
Cheers,
Wilco
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-15 17:50 ` Wilco Dijkstra
@ 2023-02-16 13:49 ` Adhemerval Zanella Netto
2023-02-16 14:02 ` Andreas Schwab
2023-02-16 18:02 ` Wilco Dijkstra
0 siblings, 2 replies; 23+ messages in thread
From: Adhemerval Zanella Netto @ 2023-02-16 13:49 UTC (permalink / raw)
To: Wilco Dijkstra, libc-alpha
On 15/02/23 14:50, Wilco Dijkstra wrote:
> Hi Adhemerval,
>
> + size_t len = __strchrnul (name, '=') - name;
>
> This is a change in behaviour, ignoring anything behind a '=' in name. What is
> wrong with strlen?
Indeed it is better to keep current semantic.
>
> + for (char **ep = __environ; *ep != NULL; ++ep)
> {
> - size_t len = strlen (name);
> -#if _STRING_ARCH_unaligned
> - name_start = *(const uint16_t *) name;
> -#else
> - name_start = (((const unsigned char *) name)[0]
> - | (((const unsigned char *) name)[1] << 8));
> -#endif
> - len -= 2;
> - name += 2;
> -
> - for (ep = __environ; *ep != NULL; ++ep)
> - {
> -#if _STRING_ARCH_unaligned
> - uint16_t ep_start = *(uint16_t *) *ep;
> -#else
> - uint16_t ep_start = (((unsigned char *) *ep)[0]
> - | (((unsigned char *) *ep)[1] << 8));
> -#endif
> -
> - if (name_start == ep_start && !strncmp (*ep + 2, name, len)
> - && (*ep)[len + 2] == '=')
> - return &(*ep)[len + 3];
> - }
> + if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
> + return *ep + len + 1;
>
> It's still useful to keep a check for the first character, eg. name[0] == (*ep)[0] as this
> is about twice as fast as always calling strncmp.
Alright, updated patch below.
From ddfb6419c5e74b34fb22454581fa0636275ec942 Mon Sep 17 00:00:00 2001
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Thu, 9 Feb 2023 10:36:57 -0300
Subject: [PATCH 2/7] stdlib: Simplify getenv
And remove _STRING_ARCH_unaligned usage.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
stdlib/getenv.c | 68 +++++++------------------------------------------
1 file changed, 9 insertions(+), 59 deletions(-)
diff --git a/stdlib/getenv.c b/stdlib/getenv.c
index e3157ce2f3..61a42d941d 100644
--- a/stdlib/getenv.c
+++ b/stdlib/getenv.c
@@ -15,76 +15,26 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <endian.h>
-#include <errno.h>
-#include <stdint.h>
+#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-
-/* Return the value of the environment variable NAME. This implementation
- is tuned a bit in that it assumes no environment variable has an empty
- name which of course should always be true. We have a special case for
- one character names so that for the general case we can assume at least
- two characters which we can access. By doing this we can avoid using the
- `strncmp' most of the time. */
char *
getenv (const char *name)
{
- char **ep;
- uint16_t name_start;
-
if (__environ == NULL || name[0] == '\0')
return NULL;
- if (name[1] == '\0')
- {
- /* The name of the variable consists of only one character. Therefore
- the first two characters of the environment entry are this character
- and a '=' character. */
-#if __BYTE_ORDER == __LITTLE_ENDIAN || !_STRING_ARCH_unaligned
- name_start = ('=' << 8) | *(const unsigned char *) name;
-#else
- name_start = '=' | ((*(const unsigned char *) name) << 8);
-#endif
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
- if (name_start == ep_start)
- return &(*ep)[2];
- }
- }
- else
- {
- size_t len = strlen (name);
-#if _STRING_ARCH_unaligned
- name_start = *(const uint16_t *) name;
-#else
- name_start = (((const unsigned char *) name)[0]
- | (((const unsigned char *) name)[1] << 8));
-#endif
- len -= 2;
- name += 2;
-
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
+ bool single_char = name[1] == '\0';
- if (name_start == ep_start && !strncmp (*ep + 2, name, len)
- && (*ep)[len + 2] == '=')
- return &(*ep)[len + 3];
- }
+ size_t len = strlen (name);;
+ for (char **ep = __environ; *ep != NULL; ++ep)
+ {
+ if (single_char && (*ep)[0] == name[0] && (*ep)[1] == '=')
+ return *ep + 2;
+ else if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
+ return *ep + len + 1;
}
return NULL;
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-16 13:49 ` Adhemerval Zanella Netto
@ 2023-02-16 14:02 ` Andreas Schwab
2023-02-16 18:02 ` Wilco Dijkstra
1 sibling, 0 replies; 23+ messages in thread
From: Andreas Schwab @ 2023-02-16 14:02 UTC (permalink / raw)
To: Adhemerval Zanella Netto via Libc-alpha
Cc: Wilco Dijkstra, Adhemerval Zanella Netto
On Feb 16 2023, Adhemerval Zanella Netto via Libc-alpha wrote:
> + size_t len = strlen (name);;
Extra semicolon.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-16 13:49 ` Adhemerval Zanella Netto
2023-02-16 14:02 ` Andreas Schwab
@ 2023-02-16 18:02 ` Wilco Dijkstra
2023-02-16 18:38 ` Adhemerval Zanella Netto
1 sibling, 1 reply; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-16 18:02 UTC (permalink / raw)
To: Adhemerval Zanella Netto, libc-alpha
Hi Adhemerval,
+ size_t len = strlen (name);;
+ for (char **ep = __environ; *ep != NULL; ++ep)
+ {
+ if (single_char && (*ep)[0] == name[0] && (*ep)[1] == '=')
+ return *ep + 2;
+ else if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
+ return *ep + len + 1;
This is about 10-20% slower than the previous patch both for single-char and
multi-char case... The approach I showed it is far simpler and 3-4 times faster
(within 10% of original performance).
Cheers,
Wilco
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-16 18:02 ` Wilco Dijkstra
@ 2023-02-16 18:38 ` Adhemerval Zanella Netto
2023-02-16 18:47 ` Wilco Dijkstra
0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella Netto @ 2023-02-16 18:38 UTC (permalink / raw)
To: Wilco Dijkstra, libc-alpha
On 16/02/23 15:02, Wilco Dijkstra wrote:
> Hi Adhemerval,
>
> + size_t len = strlen (name);;
> + for (char **ep = __environ; *ep != NULL; ++ep)
> + {
> + if (single_char && (*ep)[0] == name[0] && (*ep)[1] == '=')
> + return *ep + 2;
> + else if (strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
> + return *ep + len + 1;
>
> This is about 10-20% slower than the previous patch both for single-char and
> multi-char case... The approach I showed it is far simpler and 3-4 times faster
> (within 10% of original performance).
Do you mean something like:
size_t len = strlen (name);;
for (char **ep = __environ; *ep != NULL; ++ep)
{
if (((*ep)[0] == name[0] && (*ep)[1] == '=')
|| (strncmp (name, *ep, len) == 0 && (*ep)[len] == '='))
return *ep + len + 1;
}
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-16 18:38 ` Adhemerval Zanella Netto
@ 2023-02-16 18:47 ` Wilco Dijkstra
2023-02-16 19:29 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-16 18:47 UTC (permalink / raw)
To: Adhemerval Zanella Netto, libc-alpha
Hi Adhemerval,
> Do you mean something like:
>
> size_t len = strlen (name);;
> for (char **ep = __environ; *ep != NULL; ++ep)
> {
> if (((*ep)[0] == name[0] && (*ep)[1] == '=')
> || (strncmp (name, *ep, len) == 0 && (*ep)[len] == '='))
> return *ep + len + 1;
> }
No I meant checking the first character first before doing more expensive calls, so:
if (name[0] == (*ep)[0] && strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
return *ep + len + 1;
Basically this means you get a very tight loop checking the first character - this avoids
most calls to strncmp. This is basically the same result as the current implementation
but without all the defines and complex unaligned/big-endian stuff!
Cheers,
Wilco
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/7] stdlib: Simplify getenv
2023-02-16 18:47 ` Wilco Dijkstra
@ 2023-02-16 19:29 ` Adhemerval Zanella Netto
2023-02-17 17:13 ` Wilco Dijkstra
0 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella Netto @ 2023-02-16 19:29 UTC (permalink / raw)
To: Wilco Dijkstra, libc-alpha
On 16/02/23 15:47, Wilco Dijkstra wrote:
> Hi Adhemerval,
>
>> Do you mean something like:
>>
>> size_t len = strlen (name);;
>> for (char **ep = __environ; *ep != NULL; ++ep)
>> {
>> if (((*ep)[0] == name[0] && (*ep)[1] == '=')
>> || (strncmp (name, *ep, len) == 0 && (*ep)[len] == '='))
>> return *ep + len + 1;
>> }
>
> No I meant checking the first character first before doing more expensive calls, so:
>
> if (name[0] == (*ep)[0] && strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
> return *ep + len + 1;
>
> Basically this means you get a very tight loop checking the first character - this avoids
> most calls to strncmp. This is basically the same result as the current implementation
> but without all the defines and complex unaligned/big-endian stuff!
Ah ok, I see it now. Do the follow addresses the issues pointed out?
From 373682e6c290f1c38d0319b5695e6a12a388be0c Mon Sep 17 00:00:00 2001
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Thu, 9 Feb 2023 10:36:57 -0300
Subject: [PATCH 2/7] stdlib: Simplify getenv
And remove _STRING_ARCH_unaligned usage.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
stdlib/getenv.c | 64 ++++---------------------------------------------
1 file changed, 5 insertions(+), 59 deletions(-)
diff --git a/stdlib/getenv.c b/stdlib/getenv.c
index e3157ce2f3..6942d23433 100644
--- a/stdlib/getenv.c
+++ b/stdlib/getenv.c
@@ -15,76 +15,22 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <endian.h>
-#include <errno.h>
-#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-
-/* Return the value of the environment variable NAME. This implementation
- is tuned a bit in that it assumes no environment variable has an empty
- name which of course should always be true. We have a special case for
- one character names so that for the general case we can assume at least
- two characters which we can access. By doing this we can avoid using the
- `strncmp' most of the time. */
char *
getenv (const char *name)
{
- char **ep;
- uint16_t name_start;
-
if (__environ == NULL || name[0] == '\0')
return NULL;
- if (name[1] == '\0')
- {
- /* The name of the variable consists of only one character. Therefore
- the first two characters of the environment entry are this character
- and a '=' character. */
-#if __BYTE_ORDER == __LITTLE_ENDIAN || !_STRING_ARCH_unaligned
- name_start = ('=' << 8) | *(const unsigned char *) name;
-#else
- name_start = '=' | ((*(const unsigned char *) name) << 8);
-#endif
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
- if (name_start == ep_start)
- return &(*ep)[2];
- }
- }
- else
+ size_t len = strlen (name);;
+ for (char **ep = __environ; *ep != NULL; ++ep)
{
- size_t len = strlen (name);
-#if _STRING_ARCH_unaligned
- name_start = *(const uint16_t *) name;
-#else
- name_start = (((const unsigned char *) name)[0]
- | (((const unsigned char *) name)[1] << 8));
-#endif
- len -= 2;
- name += 2;
-
- for (ep = __environ; *ep != NULL; ++ep)
- {
-#if _STRING_ARCH_unaligned
- uint16_t ep_start = *(uint16_t *) *ep;
-#else
- uint16_t ep_start = (((unsigned char *) *ep)[0]
- | (((unsigned char *) *ep)[1] << 8));
-#endif
-
- if (name_start == ep_start && !strncmp (*ep + 2, name, len)
- && (*ep)[len + 2] == '=')
- return &(*ep)[len + 3];
- }
+ if (name[0] == (*ep)[0]
+ && strncmp (name, *ep, len) == 0 && (*ep)[len] == '=')
+ return *ep + len + 1;
}
return NULL;
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/7] nscd: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
2023-02-13 13:55 ` [PATCH 1/7] crypto: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
2023-02-13 13:55 ` [PATCH 2/7] stdlib: Simplify getenv Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-15 17:59 ` Wilco Dijkstra
2023-02-13 13:55 ` [PATCH 4/7] resolv: " Adhemerval Zanella
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
It only adds a small overhead for unaligned inputs (which should not
be common) and unify the code.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
nscd/nscd_gethst_r.c | 2 --
nscd/nscd_getserv_r.c | 2 --
nscd/nscd_helper.c | 6 ------
3 files changed, 10 deletions(-)
diff --git a/nscd/nscd_gethst_r.c b/nscd/nscd_gethst_r.c
index 7950ed695c..153194ad04 100644
--- a/nscd/nscd_gethst_r.c
+++ b/nscd/nscd_gethst_r.c
@@ -185,7 +185,6 @@ nscd_gethst_r (const char *key, size_t keylen, request_type type,
goto out;
}
-#if !_STRING_ARCH_unaligned
/* The aliases_len array in the mapped database might very
well be unaligned. We will access it word-wise so on
platforms which do not tolerate unaligned accesses we
@@ -199,7 +198,6 @@ nscd_gethst_r (const char *key, size_t keylen, request_type type,
hst_resp.h_aliases_cnt
* sizeof (uint32_t));
}
-#endif
if (type != GETHOSTBYADDR && type != GETHOSTBYNAME)
{
if (hst_resp.h_length == INADDRSZ)
diff --git a/nscd/nscd_getserv_r.c b/nscd/nscd_getserv_r.c
index 752ae1115e..0ee83ff88c 100644
--- a/nscd/nscd_getserv_r.c
+++ b/nscd/nscd_getserv_r.c
@@ -140,7 +140,6 @@ nscd_getserv_r (const char *crit, size_t critlen, const char *proto,
> recend, 0))
goto out;
-#if !_STRING_ARCH_unaligned
/* The aliases_len array in the mapped database might very
well be unaligned. We will access it word-wise so on
platforms which do not tolerate unaligned accesses we
@@ -170,7 +169,6 @@ nscd_getserv_r (const char *crit, size_t critlen, const char *proto,
serv_resp.s_aliases_cnt
* sizeof (uint32_t));
}
-#endif
}
}
diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
index fdd555ea66..6a498b363c 100644
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -465,7 +465,6 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
struct hashentry *here = (struct hashentry *) (mapped->data + work);
ref_t here_key, here_packet;
-#if !_STRING_ARCH_unaligned
/* Although during garbage collection when moving struct hashentry
records around we first copy from old to new location and then
adjust pointer from previous hashentry to it, there is no barrier
@@ -474,7 +473,6 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
application. */
if ((uintptr_t) here & (__alignof__ (*here) - 1))
return NULL;
-#endif
if (type == here->type
&& keylen == here->len
@@ -487,10 +485,8 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
struct datahead *dh
= (struct datahead *) (mapped->data + here_packet);
-#if !_STRING_ARCH_unaligned
if ((uintptr_t) dh & (__alignof__ (*dh) - 1))
return NULL;
-#endif
/* See whether we must ignore the entry or whether something
is wrong because garbage collection is in progress. */
@@ -511,11 +507,9 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
struct hashentry *trailelem;
trailelem = (struct hashentry *) (mapped->data + trail);
-#if !_STRING_ARCH_unaligned
/* We have to redo the checks. Maybe the data changed. */
if ((uintptr_t) trailelem & (__alignof__ (*trailelem) - 1))
return NULL;
-#endif
if (trail + MINIMUM_HASHENTRY_SIZE > datasize)
return NULL;
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/7] nscd: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 ` [PATCH 3/7] nscd: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
@ 2023-02-15 17:59 ` Wilco Dijkstra
0 siblings, 0 replies; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 17:59 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
This looks good - doing unaligned accesses in memory that is accessed
concurrently is a recipe for disaster since they won't be atomic on most
targets anyway.
Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
diff --git a/nscd/nscd_gethst_r.c b/nscd/nscd_gethst_r.c
index 7950ed695c..153194ad04 100644
--- a/nscd/nscd_gethst_r.c
+++ b/nscd/nscd_gethst_r.c
@@ -185,7 +185,6 @@ nscd_gethst_r (const char *key, size_t keylen, request_type type,
goto out;
}
-#if !_STRING_ARCH_unaligned
/* The aliases_len array in the mapped database might very
well be unaligned. We will access it word-wise so on
platforms which do not tolerate unaligned accesses we
@@ -199,7 +198,6 @@ nscd_gethst_r (const char *key, size_t keylen, request_type type,
hst_resp.h_aliases_cnt
* sizeof (uint32_t));
}
-#endif
if (type != GETHOSTBYADDR && type != GETHOSTBYNAME)
{
if (hst_resp.h_length == INADDRSZ)
diff --git a/nscd/nscd_getserv_r.c b/nscd/nscd_getserv_r.c
index 752ae1115e..0ee83ff88c 100644
--- a/nscd/nscd_getserv_r.c
+++ b/nscd/nscd_getserv_r.c
@@ -140,7 +140,6 @@ nscd_getserv_r (const char *crit, size_t critlen, const char *proto,
> recend, 0))
goto out;
-#if !_STRING_ARCH_unaligned
/* The aliases_len array in the mapped database might very
well be unaligned. We will access it word-wise so on
platforms which do not tolerate unaligned accesses we
@@ -170,7 +169,6 @@ nscd_getserv_r (const char *crit, size_t critlen, const char *proto,
serv_resp.s_aliases_cnt
* sizeof (uint32_t));
}
-#endif
}
}
OK
diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
index fdd555ea66..6a498b363c 100644
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -465,7 +465,6 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
struct hashentry *here = (struct hashentry *) (mapped->data + work);
ref_t here_key, here_packet;
-#if !_STRING_ARCH_unaligned
/* Although during garbage collection when moving struct hashentry
records around we first copy from old to new location and then
adjust pointer from previous hashentry to it, there is no barrier
@@ -474,7 +473,6 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
application. */
if ((uintptr_t) here & (__alignof__ (*here) - 1))
return NULL;
-#endif
if (type == here->type
&& keylen == here->len
@@ -487,10 +485,8 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
struct datahead *dh
= (struct datahead *) (mapped->data + here_packet);
-#if !_STRING_ARCH_unaligned
if ((uintptr_t) dh & (__alignof__ (*dh) - 1))
return NULL;
-#endif
/* See whether we must ignore the entry or whether something
is wrong because garbage collection is in progress. */
@@ -511,11 +507,9 @@ __nscd_cache_search (request_type type, const char *key, size_t keylen,
struct hashentry *trailelem;
trailelem = (struct hashentry *) (mapped->data + trail);
-#if !_STRING_ARCH_unaligned
/* We have to redo the checks. Maybe the data changed. */
if ((uintptr_t) trailelem & (__alignof__ (*trailelem) - 1))
return NULL;
-#endif
if (trail + MINIMUM_HASHENTRY_SIZE > datasize)
return NULL;
OK
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/7] resolv: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
` (2 preceding siblings ...)
2023-02-13 13:55 ` [PATCH 3/7] nscd: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-15 18:04 ` Wilco Dijkstra
2023-02-13 13:55 ` [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros Adhemerval Zanella
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
GCC with default implementation already generates optimized code.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
include/arpa/nameser.h | 36 ------------------------------------
1 file changed, 36 deletions(-)
diff --git a/include/arpa/nameser.h b/include/arpa/nameser.h
index c27e7886b7..0ef5ab409f 100644
--- a/include/arpa/nameser.h
+++ b/include/arpa/nameser.h
@@ -13,42 +13,6 @@
extern const struct _ns_flagdata _ns_flagdata[] attribute_hidden;
-#if _STRING_ARCH_unaligned
-
-# undef NS_GET16
-# define NS_GET16(s, cp) \
- do { \
- const uint16_t *t_cp = (const uint16_t *) (cp); \
- (s) = ntohs (*t_cp); \
- (cp) += NS_INT16SZ; \
- } while (0)
-
-# undef NS_GET32
-# define NS_GET32(l, cp) \
- do { \
- const uint32_t *t_cp = (const uint32_t *) (cp); \
- (l) = ntohl (*t_cp); \
- (cp) += NS_INT32SZ; \
- } while (0)
-
-# undef NS_PUT16
-# define NS_PUT16(s, cp) \
- do { \
- uint16_t *t_cp = (uint16_t *) (cp); \
- *t_cp = htons (s); \
- (cp) += NS_INT16SZ; \
- } while (0)
-
-# undef NS_PUT32
-# define NS_PUT32(l, cp) \
- do { \
- uint32_t *t_cp = (uint32_t *) (cp); \
- *t_cp = htonl (l); \
- (cp) += NS_INT32SZ; \
- } while (0)
-
-#endif
-
extern unsigned int __ns_get16 (const unsigned char *) __THROW;
extern unsigned long __ns_get32 (const unsigned char *) __THROW;
int __ns_name_ntop (const unsigned char *, char *, size_t) __THROW;
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 4/7] resolv: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 ` [PATCH 4/7] resolv: " Adhemerval Zanella
@ 2023-02-15 18:04 ` Wilco Dijkstra
0 siblings, 0 replies; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 18:04 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
Looks good (crazy to first define macros, then undefine and redefine
them in a different header!).
Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
diff --git a/include/arpa/nameser.h b/include/arpa/nameser.h
index c27e7886b7..0ef5ab409f 100644
--- a/include/arpa/nameser.h
+++ b/include/arpa/nameser.h
@@ -13,42 +13,6 @@
extern const struct _ns_flagdata _ns_flagdata[] attribute_hidden;
-#if _STRING_ARCH_unaligned
-
-# undef NS_GET16
-# define NS_GET16(s, cp) \
- do { \
- const uint16_t *t_cp = (const uint16_t *) (cp); \
- (s) = ntohs (*t_cp); \
- (cp) += NS_INT16SZ; \
- } while (0)
-
-# undef NS_GET32
-# define NS_GET32(l, cp) \
- do { \
- const uint32_t *t_cp = (const uint32_t *) (cp); \
- (l) = ntohl (*t_cp); \
- (cp) += NS_INT32SZ; \
- } while (0)
-
-# undef NS_PUT16
-# define NS_PUT16(s, cp) \
- do { \
- uint16_t *t_cp = (uint16_t *) (cp); \
- *t_cp = htons (s); \
- (cp) += NS_INT16SZ; \
- } while (0)
-
-# undef NS_PUT32
-# define NS_PUT32(l, cp) \
- do { \
- uint32_t *t_cp = (uint32_t *) (cp); \
- *t_cp = htonl (l); \
- (cp) += NS_INT32SZ; \
- } while (0)
-
-#endif
-
extern unsigned int __ns_get16 (const unsigned char *) __THROW;
extern unsigned long __ns_get32 (const unsigned char *) __THROW;
int __ns_name_ntop (const unsigned char *, char *, size_t) __THROW;
OK
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
` (3 preceding siblings ...)
2023-02-13 13:55 ` [PATCH 4/7] resolv: " Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-13 14:05 ` Andreas Schwab
2023-02-15 18:34 ` Wilco Dijkstra
2023-02-13 13:55 ` [PATCH 6/7] iconv: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
2023-02-13 13:55 ` [PATCH 7/7] string: Remove string_private.h Adhemerval Zanella
6 siblings, 2 replies; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
And use a packet structure instead. The compiler generates optimized
unaligned code if the architecture supports it.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
iconv/gconv_int.h | 28 ++++++++++++++
iconv/loop.c | 73 -----------------------------------
iconv/skeleton.c | 67 --------------------------------
iconvdata/iso-2022-jp-3.c | 2 +-
iconvdata/unicode.c | 6 +--
iconvdata/utf-16.c | 6 +--
iconvdata/utf-32.c | 6 +--
sysdeps/s390/utf16-utf32-z9.c | 4 +-
sysdeps/s390/utf8-utf16-z9.c | 2 +-
sysdeps/s390/utf8-utf32-z9.c | 2 +-
10 files changed, 42 insertions(+), 154 deletions(-)
diff --git a/iconv/gconv_int.h b/iconv/gconv_int.h
index da792a95f5..4b247a815f 100644
--- a/iconv/gconv_int.h
+++ b/iconv/gconv_int.h
@@ -26,6 +26,34 @@
__BEGIN_DECLS
+/* We have to provide support for machines which are not able to handled
+ unaligned memory accesses. Some of the character encodings have
+ representations with a fixed width of 2 or 4 bytes. */
+#define get16(addr) \
+({ \
+ const struct { uint16_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r; \
+})
+#define get32(addr) \
+({ \
+ const struct { uint32_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r; \
+})
+
+#define put16(addr, val) \
+do { \
+ struct { uint16_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r = val; \
+} while (0)
+#define put32(addr, val) \
+do { \
+ struct { uint32_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r = val; \
+} while (0)
/* Structure for alias definition. Simply two strings. */
struct gconv_alias
diff --git a/iconv/loop.c b/iconv/loop.c
index 963c59ca9a..9d8a7cceb3 100644
--- a/iconv/loop.c
+++ b/iconv/loop.c
@@ -57,75 +57,10 @@
#include <stddef.h>
#include <libc-diag.h>
-/* We have to provide support for machines which are not able to handled
- unaligned memory accesses. Some of the character encodings have
- representations with a fixed width of 2 or 4 bytes. But if we cannot
- access unaligned memory we still have to read byte-wise. */
#undef FCTNAME2
#if _STRING_ARCH_unaligned || !defined DEFINE_UNALIGNED
-/* We can handle unaligned memory access. */
-# define get16(addr) *((const uint16_t *) (addr))
-# define get32(addr) *((const uint32_t *) (addr))
-
-/* We need no special support for writing values either. */
-# define put16(addr, val) *((uint16_t *) (addr)) = (val)
-# define put32(addr, val) *((uint32_t *) (addr)) = (val)
-
# define FCTNAME2(name) name
#else
-/* Distinguish between big endian and little endian. */
-# if __BYTE_ORDER == __LITTLE_ENDIAN
-# define get16(addr) \
- (((const unsigned char *) (addr))[1] << 8 \
- | ((const unsigned char *) (addr))[0])
-# define get32(addr) \
- (((((const unsigned char *) (addr))[3] << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[0])
-
-# define put16(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- ((unsigned char *) (addr))[1] = __val >> 8; \
- (void) 0; })
-# define put32(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[3] = __val; \
- (void) 0; })
-# else
-# define get16(addr) \
- (((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1])
-# define get32(addr) \
- (((((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[3])
-
-# define put16(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[1] = __val; \
- ((unsigned char *) (addr))[0] = __val >> 8; \
- (void) 0; })
-# define put32(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[3] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[0] = __val; \
- (void) 0; })
-# endif
-
# define FCTNAME2(name) name##_unaligned
#endif
#define FCTNAME(name) FCTNAME2(name)
@@ -349,10 +284,6 @@ FCTNAME (LOOPFCT) (struct __gconv_step *step,
#if !defined DEFINE_UNALIGNED && !_STRING_ARCH_unaligned \
&& MIN_NEEDED_INPUT != 1 && MAX_NEEDED_INPUT % MIN_NEEDED_INPUT == 0 \
&& MIN_NEEDED_OUTPUT != 1 && MAX_NEEDED_OUTPUT % MIN_NEEDED_OUTPUT == 0
-# undef get16
-# undef get32
-# undef put16
-# undef put32
# undef unaligned
# define DEFINE_UNALIGNED
@@ -540,8 +471,4 @@ gconv_btowc (struct __gconv_step *step, unsigned char c)
#undef LOOP_NEED_STATE
#undef LOOP_NEED_FLAGS
#undef LOOP_NEED_DATA
-#undef get16
-#undef get32
-#undef put16
-#undef put32
#undef unaligned
diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 673b474134..9423d3fc5a 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -204,73 +204,6 @@
#endif
-/* Define macros which can access unaligned buffers. These macros are
- supposed to be used only in code outside the inner loops. For the inner
- loops we have other definitions which allow optimized access. */
-#if _STRING_ARCH_unaligned
-/* We can handle unaligned memory access. */
-# define get16u(addr) *((const uint16_t *) (addr))
-# define get32u(addr) *((const uint32_t *) (addr))
-
-/* We need no special support for writing values either. */
-# define put16u(addr, val) *((uint16_t *) (addr)) = (val)
-# define put32u(addr, val) *((uint32_t *) (addr)) = (val)
-#else
-/* Distinguish between big endian and little endian. */
-# if __BYTE_ORDER == __LITTLE_ENDIAN
-# define get16u(addr) \
- (((const unsigned char *) (addr))[1] << 8 \
- | ((const unsigned char *) (addr))[0])
-# define get32u(addr) \
- (((((const unsigned char *) (addr))[3] << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[0])
-
-# define put16u(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- ((unsigned char *) (addr))[1] = __val >> 8; \
- (void) 0; })
-# define put32u(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[3] = __val; \
- (void) 0; })
-# else
-# define get16u(addr) \
- (((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1])
-# define get32u(addr) \
- (((((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[3])
-
-# define put16u(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[1] = __val; \
- ((unsigned char *) (addr))[0] = __val >> 8; \
- (void) 0; })
-# define put32u(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[3] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[0] = __val; \
- (void) 0; })
-# endif
-#endif
-
-
/* For conversions from a fixed width character set to another fixed width
character set we can define RESET_INPUT_BUFFER in a very fast way. */
#if !defined RESET_INPUT_BUFFER && !defined SAVE_RESET_STATE
diff --git a/iconvdata/iso-2022-jp-3.c b/iconvdata/iso-2022-jp-3.c
index 4a4d5a3046..d341a14f51 100644
--- a/iconvdata/iso-2022-jp-3.c
+++ b/iconvdata/iso-2022-jp-3.c
@@ -91,7 +91,7 @@ enum
if (__glibc_likely (outbuf + 4 <= outend)) \
{ \
/* Write out the last character. */ \
- put32u (outbuf, ch); \
+ put32 (outbuf, ch); \
outbuf += 4; \
data->__statep->__count &= 7; \
data->__statep->__count |= ASCII_set; \
diff --git a/iconvdata/unicode.c b/iconvdata/unicode.c
index 2d131270b9..cc7999e36c 100644
--- a/iconvdata/unicode.c
+++ b/iconvdata/unicode.c
@@ -51,10 +51,10 @@
return (inptr == inend \
? __GCONV_EMPTY_INPUT : __GCONV_INCOMPLETE_INPUT); \
\
- if (get16u (inptr) == BOM) \
+ if (get16 (inptr) == BOM) \
/* Simply ignore the BOM character. */ \
*inptrp = inptr += 2; \
- else if (get16u (inptr) == BOM_OE) \
+ else if (get16 (inptr) == BOM_OE) \
{ \
data->__flags |= __GCONV_SWAP; \
*inptrp = inptr += 2; \
@@ -67,7 +67,7 @@
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM); \
+ put16 (outbuf, BOM); \
outbuf += 2; \
} \
swap = data->__flags & __GCONV_SWAP;
diff --git a/iconvdata/utf-16.c b/iconvdata/utf-16.c
index ad7dfa1a5c..edd1816c9d 100644
--- a/iconvdata/utf-16.c
+++ b/iconvdata/utf-16.c
@@ -55,10 +55,10 @@
return (inptr == inend \
? __GCONV_EMPTY_INPUT : __GCONV_INCOMPLETE_INPUT); \
\
- if (get16u (inptr) == BOM) \
+ if (get16 (inptr) == BOM) \
/* Simply ignore the BOM character. */ \
*inptrp = inptr += 2; \
- else if (get16u (inptr) == BOM_OE) \
+ else if (get16 (inptr) == BOM_OE) \
{ \
data->__flags |= __GCONV_SWAP; \
*inptrp = inptr += 2; \
@@ -70,7 +70,7 @@
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM); \
+ put16 (outbuf, BOM); \
outbuf += 2; \
} \
} \
diff --git a/iconvdata/utf-32.c b/iconvdata/utf-32.c
index 01b6d95018..41be52bb3a 100644
--- a/iconvdata/utf-32.c
+++ b/iconvdata/utf-32.c
@@ -52,10 +52,10 @@
return (inptr == inend \
? __GCONV_EMPTY_INPUT : __GCONV_INCOMPLETE_INPUT); \
\
- if (get32u (inptr) == BOM) \
+ if (get32 (inptr) == BOM) \
/* Simply ignore the BOM character. */ \
*inptrp = inptr += 4; \
- else if (get32u (inptr) == BOM_OE) \
+ else if (get32 (inptr) == BOM_OE) \
{ \
data->__flags |= __GCONV_SWAP; \
*inptrp = inptr += 4; \
@@ -69,7 +69,7 @@
if (__glibc_unlikely (outbuf + 4 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put32u (outbuf, BOM); \
+ put32 (outbuf, BOM); \
outbuf += 4; \
} \
else if (__builtin_expect (data->__invocation_counter == 0, 0) \
diff --git a/sysdeps/s390/utf16-utf32-z9.c b/sysdeps/s390/utf16-utf32-z9.c
index d87eac0bdf..36c56ccbf7 100644
--- a/sysdeps/s390/utf16-utf32-z9.c
+++ b/sysdeps/s390/utf16-utf32-z9.c
@@ -171,7 +171,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM_UTF16); \
+ put16 (outbuf, BOM_UTF16); \
outbuf += 2; \
} \
else \
@@ -180,7 +180,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 4 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put32u (outbuf, BOM_UTF32); \
+ put32 (outbuf, BOM_UTF32); \
outbuf += 4; \
} \
}
diff --git a/sysdeps/s390/utf8-utf16-z9.c b/sysdeps/s390/utf8-utf16-z9.c
index 4d5510335e..33f7c64da4 100644
--- a/sysdeps/s390/utf8-utf16-z9.c
+++ b/sysdeps/s390/utf8-utf16-z9.c
@@ -211,7 +211,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM_UTF16); \
+ put16 (outbuf, BOM_UTF16); \
outbuf += 2; \
}
diff --git a/sysdeps/s390/utf8-utf32-z9.c b/sysdeps/s390/utf8-utf32-z9.c
index c3a431d0a9..55321c519a 100644
--- a/sysdeps/s390/utf8-utf32-z9.c
+++ b/sysdeps/s390/utf8-utf32-z9.c
@@ -211,7 +211,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 4 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put32u (outbuf, BOM); \
+ put32 (outbuf, BOM); \
outbuf += 4; \
}
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros
2023-02-13 13:55 ` [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros Adhemerval Zanella
@ 2023-02-13 14:05 ` Andreas Schwab
2023-02-15 18:34 ` Wilco Dijkstra
1 sibling, 0 replies; 23+ messages in thread
From: Andreas Schwab @ 2023-02-13 14:05 UTC (permalink / raw)
To: Adhemerval Zanella via Libc-alpha; +Cc: Wilco Dijkstra, Adhemerval Zanella
On Feb 13 2023, Adhemerval Zanella via Libc-alpha wrote:
> And use a packet structure instead. The compiler generates optimized
s/packet/packed/
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros
2023-02-13 13:55 ` [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros Adhemerval Zanella
2023-02-13 14:05 ` Andreas Schwab
@ 2023-02-15 18:34 ` Wilco Dijkstra
1 sibling, 0 replies; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 18:34 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
LGTM.
Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
diff --git a/iconv/gconv_int.h b/iconv/gconv_int.h
index da792a95f5..4b247a815f 100644
--- a/iconv/gconv_int.h
+++ b/iconv/gconv_int.h
@@ -26,6 +26,34 @@
__BEGIN_DECLS
+/* We have to provide support for machines which are not able to handled
+ unaligned memory accesses. Some of the character encodings have
+ representations with a fixed width of 2 or 4 bytes. */
+#define get16(addr) \
+({ \
+ const struct { uint16_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r; \
+})
+#define get32(addr) \
+({ \
+ const struct { uint32_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r; \
+})
+
+#define put16(addr, val) \
+do { \
+ struct { uint16_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r = val; \
+} while (0)
+#define put32(addr, val) \
+do { \
+ struct { uint32_t r; } __attribute__ ((__packed__)) *__ptr \
+ = (__typeof(__ptr))(addr); \
+ __ptr->r = val; \
+} while (0)
/* Structure for alias definition. Simply two strings. */
struct gconv_alias
OK
diff --git a/iconv/loop.c b/iconv/loop.c
index 963c59ca9a..9d8a7cceb3 100644
--- a/iconv/loop.c
+++ b/iconv/loop.c
@@ -57,75 +57,10 @@
#include <stddef.h>
#include <libc-diag.h>
-/* We have to provide support for machines which are not able to handled
- unaligned memory accesses. Some of the character encodings have
- representations with a fixed width of 2 or 4 bytes. But if we cannot
- access unaligned memory we still have to read byte-wise. */
#undef FCTNAME2
#if _STRING_ARCH_unaligned || !defined DEFINE_UNALIGNED
-/* We can handle unaligned memory access. */
-# define get16(addr) *((const uint16_t *) (addr))
-# define get32(addr) *((const uint32_t *) (addr))
-
-/* We need no special support for writing values either. */
-# define put16(addr, val) *((uint16_t *) (addr)) = (val)
-# define put32(addr, val) *((uint32_t *) (addr)) = (val)
-
# define FCTNAME2(name) name
#else
-/* Distinguish between big endian and little endian. */
-# if __BYTE_ORDER == __LITTLE_ENDIAN
-# define get16(addr) \
- (((const unsigned char *) (addr))[1] << 8 \
- | ((const unsigned char *) (addr))[0])
-# define get32(addr) \
- (((((const unsigned char *) (addr))[3] << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[0])
-
-# define put16(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- ((unsigned char *) (addr))[1] = __val >> 8; \
- (void) 0; })
-# define put32(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[3] = __val; \
- (void) 0; })
-# else
-# define get16(addr) \
- (((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1])
-# define get32(addr) \
- (((((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[3])
-
-# define put16(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[1] = __val; \
- ((unsigned char *) (addr))[0] = __val >> 8; \
- (void) 0; })
-# define put32(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[3] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[0] = __val; \
- (void) 0; })
-# endif
-
# define FCTNAME2(name) name##_unaligned
#endif
#define FCTNAME(name) FCTNAME2(name)
@@ -349,10 +284,6 @@ FCTNAME (LOOPFCT) (struct __gconv_step *step,
#if !defined DEFINE_UNALIGNED && !_STRING_ARCH_unaligned \
&& MIN_NEEDED_INPUT != 1 && MAX_NEEDED_INPUT % MIN_NEEDED_INPUT == 0 \
&& MIN_NEEDED_OUTPUT != 1 && MAX_NEEDED_OUTPUT % MIN_NEEDED_OUTPUT == 0
-# undef get16
-# undef get32
-# undef put16
-# undef put32
# undef unaligned
# define DEFINE_UNALIGNED
@@ -540,8 +471,4 @@ gconv_btowc (struct __gconv_step *step, unsigned char c)
#undef LOOP_NEED_STATE
#undef LOOP_NEED_FLAGS
#undef LOOP_NEED_DATA
-#undef get16
-#undef get32
-#undef put16
-#undef put32
#undef unaligned
OK
diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 673b474134..9423d3fc5a 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -204,73 +204,6 @@
#endif
-/* Define macros which can access unaligned buffers. These macros are
- supposed to be used only in code outside the inner loops. For the inner
- loops we have other definitions which allow optimized access. */
-#if _STRING_ARCH_unaligned
-/* We can handle unaligned memory access. */
-# define get16u(addr) *((const uint16_t *) (addr))
-# define get32u(addr) *((const uint32_t *) (addr))
-
-/* We need no special support for writing values either. */
-# define put16u(addr, val) *((uint16_t *) (addr)) = (val)
-# define put32u(addr, val) *((uint32_t *) (addr)) = (val)
-#else
-/* Distinguish between big endian and little endian. */
-# if __BYTE_ORDER == __LITTLE_ENDIAN
-# define get16u(addr) \
- (((const unsigned char *) (addr))[1] << 8 \
- | ((const unsigned char *) (addr))[0])
-# define get32u(addr) \
- (((((const unsigned char *) (addr))[3] << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[0])
-
-# define put16u(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- ((unsigned char *) (addr))[1] = __val >> 8; \
- (void) 0; })
-# define put32u(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[0] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[3] = __val; \
- (void) 0; })
-# else
-# define get16u(addr) \
- (((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1])
-# define get32u(addr) \
- (((((const unsigned char *) (addr))[0] << 8 \
- | ((const unsigned char *) (addr))[1]) << 8 \
- | ((const unsigned char *) (addr))[2]) << 8 \
- | ((const unsigned char *) (addr))[3])
-
-# define put16u(addr, val) \
- ({ uint16_t __val = (val); \
- ((unsigned char *) (addr))[1] = __val; \
- ((unsigned char *) (addr))[0] = __val >> 8; \
- (void) 0; })
-# define put32u(addr, val) \
- ({ uint32_t __val = (val); \
- ((unsigned char *) (addr))[3] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[2] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[1] = __val; \
- __val >>= 8; \
- ((unsigned char *) (addr))[0] = __val; \
- (void) 0; })
-# endif
-#endif
-
-
/* For conversions from a fixed width character set to another fixed width
character set we can define RESET_INPUT_BUFFER in a very fast way. */
#if !defined RESET_INPUT_BUFFER && !defined SAVE_RESET_STATE
OK
diff --git a/iconvdata/iso-2022-jp-3.c b/iconvdata/iso-2022-jp-3.c
index 4a4d5a3046..d341a14f51 100644
--- a/iconvdata/iso-2022-jp-3.c
+++ b/iconvdata/iso-2022-jp-3.c
@@ -91,7 +91,7 @@ enum
if (__glibc_likely (outbuf + 4 <= outend)) \
{ \
/* Write out the last character. */ \
- put32u (outbuf, ch); \
+ put32 (outbuf, ch); \
outbuf += 4; \
data->__statep->__count &= 7; \
data->__statep->__count |= ASCII_set; \
OK
diff --git a/iconvdata/unicode.c b/iconvdata/unicode.c
index 2d131270b9..cc7999e36c 100644
--- a/iconvdata/unicode.c
+++ b/iconvdata/unicode.c
@@ -51,10 +51,10 @@
return (inptr == inend \
? __GCONV_EMPTY_INPUT : __GCONV_INCOMPLETE_INPUT); \
\
- if (get16u (inptr) == BOM) \
+ if (get16 (inptr) == BOM) \
/* Simply ignore the BOM character. */ \
*inptrp = inptr += 2; \
- else if (get16u (inptr) == BOM_OE) \
+ else if (get16 (inptr) == BOM_OE) \
{ \
data->__flags |= __GCONV_SWAP; \
*inptrp = inptr += 2; \
@@ -67,7 +67,7 @@
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM); \
+ put16 (outbuf, BOM); \
outbuf += 2; \
} \
swap = data->__flags & __GCONV_SWAP;
diff --git a/iconvdata/utf-16.c b/iconvdata/utf-16.c
index ad7dfa1a5c..edd1816c9d 100644
--- a/iconvdata/utf-16.c
+++ b/iconvdata/utf-16.c
@@ -55,10 +55,10 @@
return (inptr == inend \
? __GCONV_EMPTY_INPUT : __GCONV_INCOMPLETE_INPUT); \
\
- if (get16u (inptr) == BOM) \
+ if (get16 (inptr) == BOM) \
/* Simply ignore the BOM character. */ \
*inptrp = inptr += 2; \
- else if (get16u (inptr) == BOM_OE) \
+ else if (get16 (inptr) == BOM_OE) \
{ \
data->__flags |= __GCONV_SWAP; \
*inptrp = inptr += 2; \
@@ -70,7 +70,7 @@
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM); \
+ put16 (outbuf, BOM); \
outbuf += 2; \
} \
} \
diff --git a/iconvdata/utf-32.c b/iconvdata/utf-32.c
index 01b6d95018..41be52bb3a 100644
--- a/iconvdata/utf-32.c
+++ b/iconvdata/utf-32.c
@@ -52,10 +52,10 @@
return (inptr == inend \
? __GCONV_EMPTY_INPUT : __GCONV_INCOMPLETE_INPUT); \
\
- if (get32u (inptr) == BOM) \
+ if (get32 (inptr) == BOM) \
/* Simply ignore the BOM character. */ \
*inptrp = inptr += 4; \
- else if (get32u (inptr) == BOM_OE) \
+ else if (get32 (inptr) == BOM_OE) \
{ \
data->__flags |= __GCONV_SWAP; \
*inptrp = inptr += 4; \
@@ -69,7 +69,7 @@
if (__glibc_unlikely (outbuf + 4 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put32u (outbuf, BOM); \
+ put32 (outbuf, BOM); \
outbuf += 4; \
} \
else if (__builtin_expect (data->__invocation_counter == 0, 0) \
OK
diff --git a/sysdeps/s390/utf16-utf32-z9.c b/sysdeps/s390/utf16-utf32-z9.c
index d87eac0bdf..36c56ccbf7 100644
--- a/sysdeps/s390/utf16-utf32-z9.c
+++ b/sysdeps/s390/utf16-utf32-z9.c
@@ -171,7 +171,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM_UTF16); \
+ put16 (outbuf, BOM_UTF16); \
outbuf += 2; \
} \
else \
@@ -180,7 +180,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 4 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put32u (outbuf, BOM_UTF32); \
+ put32 (outbuf, BOM_UTF32); \
outbuf += 4; \
} \
}
diff --git a/sysdeps/s390/utf8-utf16-z9.c b/sysdeps/s390/utf8-utf16-z9.c
index 4d5510335e..33f7c64da4 100644
--- a/sysdeps/s390/utf8-utf16-z9.c
+++ b/sysdeps/s390/utf8-utf16-z9.c
@@ -211,7 +211,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 2 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put16u (outbuf, BOM_UTF16); \
+ put16 (outbuf, BOM_UTF16); \
outbuf += 2; \
}
diff --git a/sysdeps/s390/utf8-utf32-z9.c b/sysdeps/s390/utf8-utf32-z9.c
index c3a431d0a9..55321c519a 100644
--- a/sysdeps/s390/utf8-utf32-z9.c
+++ b/sysdeps/s390/utf8-utf32-z9.c
@@ -211,7 +211,7 @@ gconv_end (struct __gconv_step *data)
if (__glibc_unlikely (outbuf + 4 > outend)) \
return __GCONV_FULL_OUTPUT; \
\
- put32u (outbuf, BOM); \
+ put32 (outbuf, BOM); \
outbuf += 4; \
}
OK
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 6/7] iconv: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
` (4 preceding siblings ...)
2023-02-13 13:55 ` [PATCH 5/7] iconv: Remove _STRING_ARCH_unaligned usage for get/set macros Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-15 19:02 ` Wilco Dijkstra
2023-02-13 13:55 ` [PATCH 7/7] string: Remove string_private.h Adhemerval Zanella
6 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
Use put/get macros __builtin_bswap32 instead. It allows to remove
the unaligned routines, the compiler will generate unaligned access
if the ABI allows it.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
iconv/gconv_simple.c | 282 +++----------------------------------------
iconv/loop.c | 66 ++++------
iconv/skeleton.c | 118 +++---------------
3 files changed, 59 insertions(+), 407 deletions(-)
diff --git a/iconv/gconv_simple.c b/iconv/gconv_simple.c
index c50ffd3bf0..c60cffad4c 100644
--- a/iconv/gconv_simple.c
+++ b/iconv/gconv_simple.c
@@ -86,69 +86,22 @@ internal_ucs4_loop (struct __gconv_step *step,
#if __BYTE_ORDER == __LITTLE_ENDIAN
/* Sigh, we have to do some real work. */
size_t cnt;
- uint32_t *outptr32 = (uint32_t *) outptr;
-
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
- *outptr32++ = bswap_32 (*(const uint32_t *) inptr);
-
- *inptrp = inptr;
- *outptrp = (unsigned char *) outptr32;
-#elif __BYTE_ORDER == __BIG_ENDIAN
- /* Simply copy the data. */
- *inptrp = inptr + n_convert * 4;
- *outptrp = __mempcpy (outptr, inptr, n_convert * 4);
-#else
-# error "This endianess is not supported."
-#endif
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*outptrp + 4 > outend)
- result = __GCONV_FULL_OUTPUT;
- else
- result = __GCONV_INCOMPLETE_INPUT;
-
- return result;
-}
-
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-internal_ucs4_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
- int result;
-
-# if __BYTE_ORDER == __LITTLE_ENDIAN
- /* Sigh, we have to do some real work. */
- size_t cnt;
for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4, outptr += 4)
{
- outptr[0] = inptr[3];
- outptr[1] = inptr[2];
- outptr[2] = inptr[1];
- outptr[3] = inptr[0];
+ uint32_t val = get32 (inptr);
+ put32 (outptr, __builtin_bswap32 (val));
}
*inptrp = inptr;
*outptrp = outptr;
-# elif __BYTE_ORDER == __BIG_ENDIAN
+#elif __BYTE_ORDER == __BIG_ENDIAN
/* Simply copy the data. */
*inptrp = inptr + n_convert * 4;
*outptrp = __mempcpy (outptr, inptr, n_convert * 4);
-# else
-# error "This endianess is not supported."
-# endif
+#else
+# error "This endianess is not supported."
+#endif
/* Determine the status. */
if (*inptrp == inend)
@@ -160,7 +113,6 @@ internal_ucs4_loop_unaligned (struct __gconv_step *step,
return result;
}
-#endif
static inline int
@@ -242,12 +194,9 @@ ucs4_internal_loop (struct __gconv_step *step,
for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
- uint32_t inval;
-
+ uint32_t inval = get32 (inptr);
#if __BYTE_ORDER == __LITTLE_ENDIAN
- inval = bswap_32 (*(const uint32_t *) inptr);
-#else
- inval = *(const uint32_t *) inptr;
+ inval = __builtin_bswap32 (inval);
#endif
if (__glibc_unlikely (inval > 0x7fffffff))
@@ -272,7 +221,7 @@ ucs4_internal_loop (struct __gconv_step *step,
return __GCONV_ILLEGAL_INPUT;
}
- *((uint32_t *) outptr) = inval;
+ put32 (outptr, inval);
outptr += sizeof (uint32_t);
}
@@ -290,75 +239,6 @@ ucs4_internal_loop (struct __gconv_step *step,
return result;
}
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-ucs4_internal_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- int flags = step_data->__flags;
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- int result;
-
- for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
- {
- if (__glibc_unlikely (inptr[0] > 0x80))
- {
- /* The value is too large. We don't try transliteration here since
- this is not an error because of the lack of possibilities to
- represent the result. This is a genuine bug in the input since
- UCS4 does not allow such values. */
- if (irreversible == NULL)
- /* We are transliterating, don't try to correct anything. */
- return __GCONV_ILLEGAL_INPUT;
-
- if (flags & __GCONV_IGNORE_ERRORS)
- {
- /* Just ignore this character. */
- ++*irreversible;
- continue;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
- return __GCONV_ILLEGAL_INPUT;
- }
-
-# if __BYTE_ORDER == __LITTLE_ENDIAN
- outptr[3] = inptr[0];
- outptr[2] = inptr[1];
- outptr[1] = inptr[2];
- outptr[0] = inptr[3];
-# else
- outptr[0] = inptr[0];
- outptr[1] = inptr[1];
- outptr[2] = inptr[2];
- outptr[3] = inptr[3];
-# endif
- outptr += 4;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*outptrp + 4 > outend)
- result = __GCONV_FULL_OUTPUT;
- else
- result = __GCONV_INCOMPLETE_INPUT;
-
- return result;
-}
-#endif
-
static inline int
__attribute ((always_inline))
@@ -453,11 +333,12 @@ internal_ucs4le_loop (struct __gconv_step *step,
#if __BYTE_ORDER == __BIG_ENDIAN
/* Sigh, we have to do some real work. */
size_t cnt;
- uint32_t *outptr32 = (uint32_t *) outptr;
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
- *outptr32++ = bswap_32 (*(const uint32_t *) inptr);
- outptr = (unsigned char *) outptr32;
+ for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4, outptr += 4)
+ {
+ uint32_t val = get32 (inptr);
+ put32 (outptr, __builtin_bswap32 (val));
+ }
*inptrp = inptr;
*outptrp = outptr;
@@ -480,59 +361,6 @@ internal_ucs4le_loop (struct __gconv_step *step,
return result;
}
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-internal_ucs4le_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
- int result;
-
-# if __BYTE_ORDER == __BIG_ENDIAN
- /* Sigh, we have to do some real work. */
- size_t cnt;
-
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4, outptr += 4)
- {
- outptr[0] = inptr[3];
- outptr[1] = inptr[2];
- outptr[2] = inptr[1];
- outptr[3] = inptr[0];
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
-# elif __BYTE_ORDER == __LITTLE_ENDIAN
- /* Simply copy the data. */
- *inptrp = inptr + n_convert * 4;
- *outptrp = __mempcpy (outptr, inptr, n_convert * 4);
-# else
-# error "This endianess is not supported."
-# endif
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*inptrp + 4 > inend)
- result = __GCONV_INCOMPLETE_INPUT;
- else
- {
- assert (*outptrp + 4 > outend);
- result = __GCONV_FULL_OUTPUT;
- }
-
- return result;
-}
-#endif
-
static inline int
__attribute ((always_inline))
@@ -612,12 +440,9 @@ ucs4le_internal_loop (struct __gconv_step *step,
for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
- uint32_t inval;
-
+ uint32_t inval = get32 (inptr);
#if __BYTE_ORDER == __BIG_ENDIAN
- inval = bswap_32 (*(const uint32_t *) inptr);
-#else
- inval = *(const uint32_t *) inptr;
+ inval = __builtin_bswap32 (inval);
#endif
if (__glibc_unlikely (inval > 0x7fffffff))
@@ -642,7 +467,7 @@ ucs4le_internal_loop (struct __gconv_step *step,
return __GCONV_ILLEGAL_INPUT;
}
- *((uint32_t *) outptr) = inval;
+ put32 (outptr, inval);
outptr += sizeof (uint32_t);
}
@@ -663,79 +488,6 @@ ucs4le_internal_loop (struct __gconv_step *step,
return result;
}
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-ucs4le_internal_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- int flags = step_data->__flags;
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- int result;
-
- for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
- {
- if (__glibc_unlikely (inptr[3] > 0x80))
- {
- /* The value is too large. We don't try transliteration here since
- this is not an error because of the lack of possibilities to
- represent the result. This is a genuine bug in the input since
- UCS4 does not allow such values. */
- if (irreversible == NULL)
- /* We are transliterating, don't try to correct anything. */
- return __GCONV_ILLEGAL_INPUT;
-
- if (flags & __GCONV_IGNORE_ERRORS)
- {
- /* Just ignore this character. */
- ++*irreversible;
- continue;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
- return __GCONV_ILLEGAL_INPUT;
- }
-
-# if __BYTE_ORDER == __BIG_ENDIAN
- outptr[3] = inptr[0];
- outptr[2] = inptr[1];
- outptr[1] = inptr[2];
- outptr[0] = inptr[3];
-# else
- outptr[0] = inptr[0];
- outptr[1] = inptr[1];
- outptr[2] = inptr[2];
- outptr[3] = inptr[3];
-# endif
-
- outptr += 4;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*inptrp + 4 > inend)
- result = __GCONV_INCOMPLETE_INPUT;
- else
- {
- assert (*outptrp + 4 > outend);
- result = __GCONV_FULL_OUTPUT;
- }
-
- return result;
-}
-#endif
-
static inline int
__attribute ((always_inline))
diff --git a/iconv/loop.c b/iconv/loop.c
index 9d8a7cceb3..b2a1727ad4 100644
--- a/iconv/loop.c
+++ b/iconv/loop.c
@@ -58,12 +58,7 @@
#include <libc-diag.h>
#undef FCTNAME2
-#if _STRING_ARCH_unaligned || !defined DEFINE_UNALIGNED
-# define FCTNAME2(name) name
-#else
-# define FCTNAME2(name) name##_unaligned
-#endif
-#define FCTNAME(name) FCTNAME2(name)
+#define FCTNAME(name) name
/* We need at least one byte for the next round. */
@@ -279,20 +274,9 @@ FCTNAME (LOOPFCT) (struct __gconv_step *step,
}
-/* Include the file a second time to define the function to handle
- unaligned access. */
-#if !defined DEFINE_UNALIGNED && !_STRING_ARCH_unaligned \
- && MIN_NEEDED_INPUT != 1 && MAX_NEEDED_INPUT % MIN_NEEDED_INPUT == 0 \
- && MIN_NEEDED_OUTPUT != 1 && MAX_NEEDED_OUTPUT % MIN_NEEDED_OUTPUT == 0
-# undef unaligned
-
-# define DEFINE_UNALIGNED
-# include "loop.c"
-# undef DEFINE_UNALIGNED
-#else
-# if MAX_NEEDED_INPUT > 1
-# define SINGLE(fct) SINGLE2 (fct)
-# define SINGLE2(fct) fct##_single
+#if MAX_NEEDED_INPUT > 1
+# define SINGLE(fct) SINGLE2 (fct)
+# define SINGLE2(fct) fct##_single
static inline int
__attribute ((always_inline))
SINGLE(LOOPFCT) (struct __gconv_step *step,
@@ -302,37 +286,37 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
size_t *irreversible EXTRA_LOOP_DECLS)
{
mbstate_t *state = step_data->__statep;
-# ifdef LOOP_NEED_FLAGS
+# ifdef LOOP_NEED_FLAGS
int flags = step_data->__flags;
-# endif
-# ifdef LOOP_NEED_DATA
+# endif
+# ifdef LOOP_NEED_DATA
void *data = step->__data;
-# endif
+# endif
int result = __GCONV_OK;
unsigned char bytebuf[MAX_NEEDED_INPUT];
const unsigned char *inptr = *inptrp;
unsigned char *outptr = *outptrp;
size_t inlen;
-# ifdef INIT_PARAMS
+# ifdef INIT_PARAMS
INIT_PARAMS;
-# endif
+# endif
-# ifdef UNPACK_BYTES
+# ifdef UNPACK_BYTES
UNPACK_BYTES
-# else
+# else
/* Add the bytes from the state to the input buffer. */
assert ((state->__count & 7) <= sizeof (state->__value));
for (inlen = 0; inlen < (size_t) (state->__count & 7); ++inlen)
bytebuf[inlen] = state->__value.__wchb[inlen];
-# endif
+# endif
/* Are there enough bytes in the input buffer? */
if (MIN_NEEDED_INPUT > 1
&& __builtin_expect (inptr + (MIN_NEEDED_INPUT - inlen) > inend, 0))
{
*inptrp = inend;
-# ifdef STORE_REST
+# ifdef STORE_REST
/* Building with -O3 GCC emits a `array subscript is above array
bounds' warning. GCC BZ #64739 has been opened for this. */
@@ -347,14 +331,14 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
inend = &bytebuf[inlen];
STORE_REST
-# else
+# else
/* We don't have enough input for another complete input
character. */
size_t inlen_after = inlen + (inend - inptr);
assert (inlen_after <= sizeof (state->__value.__wchb));
for (; inlen < inlen_after; inlen++)
state->__value.__wchb[inlen] = *inptr++;
-# endif
+# endif
return __GCONV_INCOMPLETE_INPUT;
}
@@ -406,11 +390,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
result = __GCONV_OK;
/* Clear the state buffer. */
-# ifdef CLEAR_STATE
+# ifdef CLEAR_STATE
CLEAR_STATE;
-# else
+# else
state->__count &= ~7;
-# endif
+# endif
}
else if (result == __GCONV_INCOMPLETE_INPUT)
{
@@ -419,11 +403,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
assert (inend != &bytebuf[MAX_NEEDED_INPUT]);
*inptrp += inend - bytebuf - (state->__count & 7);
-# ifdef STORE_REST
+# ifdef STORE_REST
inptrp = &inptr;
STORE_REST
-# else
+# else
/* We don't have enough input for another complete input
character. */
assert (inend - inptr > (state->__count & ~7));
@@ -432,14 +416,13 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
for (inlen = 0; inlen < inend - inptr; inlen++)
state->__value.__wchb[inlen] = inptr[inlen];
inptr = inend;
-# endif
+# endif
}
return result;
}
-# undef SINGLE
-# undef SINGLE2
-# endif
+# undef SINGLE
+# undef SINGLE2
# ifdef ONEBYTE_BODY
@@ -471,4 +454,3 @@ gconv_btowc (struct __gconv_step *step, unsigned char c)
#undef LOOP_NEED_STATE
#undef LOOP_NEED_FLAGS
#undef LOOP_NEED_DATA
-#undef unaligned
diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 9423d3fc5a..61cff234ac 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -448,33 +448,6 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
size_t lirreversible = 0;
size_t *lirreversiblep = irreversible ? &lirreversible : NULL;
- /* The following assumes that encodings, which have a variable length
- what might unalign a buffer even though it is an aligned in the
- beginning, either don't have the minimal number of bytes as a divisor
- of the maximum length or have a minimum length of 1. This is true
- for all known and supported encodings.
- We use && instead of || to combine the subexpression for the FROM
- encoding and for the TO encoding, because usually one of them is
- INTERNAL, for which the subexpression evaluates to 1, but INTERNAL
- buffers are always aligned correctly. */
-#define POSSIBLY_UNALIGNED \
- (!_STRING_ARCH_unaligned \
- && (((FROM_LOOP_MIN_NEEDED_FROM != 1 \
- && FROM_LOOP_MAX_NEEDED_FROM % FROM_LOOP_MIN_NEEDED_FROM == 0) \
- && (FROM_LOOP_MIN_NEEDED_TO != 1 \
- && FROM_LOOP_MAX_NEEDED_TO % FROM_LOOP_MIN_NEEDED_TO == 0)) \
- || ((TO_LOOP_MIN_NEEDED_FROM != 1 \
- && TO_LOOP_MAX_NEEDED_FROM % TO_LOOP_MIN_NEEDED_FROM == 0) \
- && (TO_LOOP_MIN_NEEDED_TO != 1 \
- && TO_LOOP_MAX_NEEDED_TO % TO_LOOP_MIN_NEEDED_TO == 0))))
-#if POSSIBLY_UNALIGNED
- int unaligned;
-# define GEN_unaligned(name) GEN_unaligned2 (name)
-# define GEN_unaligned2(name) name##_unaligned
-#else
-# define unaligned 0
-#endif
-
#ifdef PREPARE_LOOP
PREPARE_LOOP
#endif
@@ -514,18 +487,6 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
}
#endif
-#if POSSIBLY_UNALIGNED
- unaligned =
- ((FROM_DIRECTION
- && ((uintptr_t) inptr % FROM_LOOP_MIN_NEEDED_FROM != 0
- || ((data->__flags & __GCONV_IS_LAST)
- && (uintptr_t) outbuf % FROM_LOOP_MIN_NEEDED_TO != 0)))
- || (!FROM_DIRECTION
- && (((data->__flags & __GCONV_IS_LAST)
- && (uintptr_t) outbuf % TO_LOOP_MIN_NEEDED_TO != 0)
- || (uintptr_t) inptr % TO_LOOP_MIN_NEEDED_FROM != 0)));
-#endif
-
while (1)
{
/* Remember the start value for this round. */
@@ -543,34 +504,14 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
SAVE_RESET_STATE (1);
#endif
- if (__glibc_likely (!unaligned))
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- status = FROM_LOOP (step, data, inptrp, inend, &outbuf, outend,
- lirreversiblep EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- status = TO_LOOP (step, data, inptrp, inend, &outbuf, outend,
- lirreversiblep EXTRA_LOOP_ARGS);
- }
-#if POSSIBLY_UNALIGNED
+ if (FROM_DIRECTION)
+ /* Run the conversion loop. */
+ status = FROM_LOOP (step, data, inptrp, inend, &outbuf, outend,
+ lirreversiblep EXTRA_LOOP_ARGS);
else
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- status = GEN_unaligned (FROM_LOOP) (step, data, inptrp, inend,
- &outbuf, outend,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- status = GEN_unaligned (TO_LOOP) (step, data, inptrp, inend,
- &outbuf, outend,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- }
-#endif
+ /* Run the conversion loop. */
+ status = TO_LOOP (step, data, inptrp, inend, &outbuf, outend,
+ lirreversiblep EXTRA_LOOP_ARGS);
/* If we were called as part of an error handling module we
don't do anything else here. */
@@ -635,41 +576,18 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
SAVE_RESET_STATE (0);
#endif
- if (__glibc_likely (!unaligned))
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- nstatus = FROM_LOOP (step, data, inptrp, inend,
- &outbuf, outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- nstatus = TO_LOOP (step, data, inptrp, inend,
- &outbuf, outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- }
-#if POSSIBLY_UNALIGNED
+ if (FROM_DIRECTION)
+ /* Run the conversion loop. */
+ nstatus = FROM_LOOP (step, data, inptrp, inend,
+ &outbuf, outerr,
+ lirreversiblep
+ EXTRA_LOOP_ARGS);
else
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- nstatus = GEN_unaligned (FROM_LOOP) (step, data,
- inptrp, inend,
- &outbuf,
- outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- nstatus = GEN_unaligned (TO_LOOP) (step, data,
- inptrp, inend,
- &outbuf, outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- }
-#endif
+ /* Run the conversion loop. */
+ nstatus = TO_LOOP (step, data, inptrp, inend,
+ &outbuf, outerr,
+ lirreversiblep
+ EXTRA_LOOP_ARGS);
/* We must run out of output buffer space in this
rerun. */
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 6/7] iconv: Remove _STRING_ARCH_unaligned usage
2023-02-13 13:55 ` [PATCH 6/7] iconv: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
@ 2023-02-15 19:02 ` Wilco Dijkstra
0 siblings, 0 replies; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 19:02 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
What a mess that was, great cleanup! LGTM.
Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
diff --git a/iconv/gconv_simple.c b/iconv/gconv_simple.c
index c50ffd3bf0..c60cffad4c 100644
--- a/iconv/gconv_simple.c
+++ b/iconv/gconv_simple.c
@@ -86,69 +86,22 @@ internal_ucs4_loop (struct __gconv_step *step,
#if __BYTE_ORDER == __LITTLE_ENDIAN
/* Sigh, we have to do some real work. */
size_t cnt;
- uint32_t *outptr32 = (uint32_t *) outptr;
-
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
- *outptr32++ = bswap_32 (*(const uint32_t *) inptr);
-
- *inptrp = inptr;
- *outptrp = (unsigned char *) outptr32;
-#elif __BYTE_ORDER == __BIG_ENDIAN
- /* Simply copy the data. */
- *inptrp = inptr + n_convert * 4;
- *outptrp = __mempcpy (outptr, inptr, n_convert * 4);
-#else
-# error "This endianess is not supported."
-#endif
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*outptrp + 4 > outend)
- result = __GCONV_FULL_OUTPUT;
- else
- result = __GCONV_INCOMPLETE_INPUT;
-
- return result;
-}
-
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-internal_ucs4_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
- int result;
-
-# if __BYTE_ORDER == __LITTLE_ENDIAN
- /* Sigh, we have to do some real work. */
- size_t cnt;
for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4, outptr += 4)
{
- outptr[0] = inptr[3];
- outptr[1] = inptr[2];
- outptr[2] = inptr[1];
- outptr[3] = inptr[0];
+ uint32_t val = get32 (inptr);
+ put32 (outptr, __builtin_bswap32 (val));
}
*inptrp = inptr;
*outptrp = outptr;
-# elif __BYTE_ORDER == __BIG_ENDIAN
+#elif __BYTE_ORDER == __BIG_ENDIAN
/* Simply copy the data. */
*inptrp = inptr + n_convert * 4;
*outptrp = __mempcpy (outptr, inptr, n_convert * 4);
-# else
-# error "This endianess is not supported."
-# endif
+#else
+# error "This endianess is not supported."
+#endif
/* Determine the status. */
if (*inptrp == inend)
@@ -160,7 +113,6 @@ internal_ucs4_loop_unaligned (struct __gconv_step *step,
return result;
}
-#endif
static inline int
@@ -242,12 +194,9 @@ ucs4_internal_loop (struct __gconv_step *step,
for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
- uint32_t inval;
-
+ uint32_t inval = get32 (inptr);
#if __BYTE_ORDER == __LITTLE_ENDIAN
- inval = bswap_32 (*(const uint32_t *) inptr);
-#else
- inval = *(const uint32_t *) inptr;
+ inval = __builtin_bswap32 (inval);
#endif
if (__glibc_unlikely (inval > 0x7fffffff))
@@ -272,7 +221,7 @@ ucs4_internal_loop (struct __gconv_step *step,
return __GCONV_ILLEGAL_INPUT;
}
- *((uint32_t *) outptr) = inval;
+ put32 (outptr, inval);
outptr += sizeof (uint32_t);
}
@@ -290,75 +239,6 @@ ucs4_internal_loop (struct __gconv_step *step,
return result;
}
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-ucs4_internal_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- int flags = step_data->__flags;
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- int result;
-
- for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
- {
- if (__glibc_unlikely (inptr[0] > 0x80))
- {
- /* The value is too large. We don't try transliteration here since
- this is not an error because of the lack of possibilities to
- represent the result. This is a genuine bug in the input since
- UCS4 does not allow such values. */
- if (irreversible == NULL)
- /* We are transliterating, don't try to correct anything. */
- return __GCONV_ILLEGAL_INPUT;
-
- if (flags & __GCONV_IGNORE_ERRORS)
- {
- /* Just ignore this character. */
- ++*irreversible;
- continue;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
- return __GCONV_ILLEGAL_INPUT;
- }
-
-# if __BYTE_ORDER == __LITTLE_ENDIAN
- outptr[3] = inptr[0];
- outptr[2] = inptr[1];
- outptr[1] = inptr[2];
- outptr[0] = inptr[3];
-# else
- outptr[0] = inptr[0];
- outptr[1] = inptr[1];
- outptr[2] = inptr[2];
- outptr[3] = inptr[3];
-# endif
- outptr += 4;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*outptrp + 4 > outend)
- result = __GCONV_FULL_OUTPUT;
- else
- result = __GCONV_INCOMPLETE_INPUT;
-
- return result;
-}
-#endif
-
static inline int
__attribute ((always_inline))
@@ -453,11 +333,12 @@ internal_ucs4le_loop (struct __gconv_step *step,
#if __BYTE_ORDER == __BIG_ENDIAN
/* Sigh, we have to do some real work. */
size_t cnt;
- uint32_t *outptr32 = (uint32_t *) outptr;
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
- *outptr32++ = bswap_32 (*(const uint32_t *) inptr);
- outptr = (unsigned char *) outptr32;
+ for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4, outptr += 4)
+ {
+ uint32_t val = get32 (inptr);
+ put32 (outptr, __builtin_bswap32 (val));
+ }
*inptrp = inptr;
*outptrp = outptr;
@@ -480,59 +361,6 @@ internal_ucs4le_loop (struct __gconv_step *step,
return result;
}
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-internal_ucs4le_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
- int result;
-
-# if __BYTE_ORDER == __BIG_ENDIAN
- /* Sigh, we have to do some real work. */
- size_t cnt;
-
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4, outptr += 4)
- {
- outptr[0] = inptr[3];
- outptr[1] = inptr[2];
- outptr[2] = inptr[1];
- outptr[3] = inptr[0];
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
-# elif __BYTE_ORDER == __LITTLE_ENDIAN
- /* Simply copy the data. */
- *inptrp = inptr + n_convert * 4;
- *outptrp = __mempcpy (outptr, inptr, n_convert * 4);
-# else
-# error "This endianess is not supported."
-# endif
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*inptrp + 4 > inend)
- result = __GCONV_INCOMPLETE_INPUT;
- else
- {
- assert (*outptrp + 4 > outend);
- result = __GCONV_FULL_OUTPUT;
- }
-
- return result;
-}
-#endif
-
static inline int
__attribute ((always_inline))
@@ -612,12 +440,9 @@ ucs4le_internal_loop (struct __gconv_step *step,
for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
- uint32_t inval;
-
+ uint32_t inval = get32 (inptr);
#if __BYTE_ORDER == __BIG_ENDIAN
- inval = bswap_32 (*(const uint32_t *) inptr);
-#else
- inval = *(const uint32_t *) inptr;
+ inval = __builtin_bswap32 (inval);
#endif
if (__glibc_unlikely (inval > 0x7fffffff))
@@ -642,7 +467,7 @@ ucs4le_internal_loop (struct __gconv_step *step,
return __GCONV_ILLEGAL_INPUT;
}
- *((uint32_t *) outptr) = inval;
+ put32 (outptr, inval);
outptr += sizeof (uint32_t);
}
@@ -663,79 +488,6 @@ ucs4le_internal_loop (struct __gconv_step *step,
return result;
}
-#if !_STRING_ARCH_unaligned
-static inline int
-__attribute ((always_inline))
-ucs4le_internal_loop_unaligned (struct __gconv_step *step,
- struct __gconv_step_data *step_data,
- const unsigned char **inptrp,
- const unsigned char *inend,
- unsigned char **outptrp,
- const unsigned char *outend,
- size_t *irreversible)
-{
- int flags = step_data->__flags;
- const unsigned char *inptr = *inptrp;
- unsigned char *outptr = *outptrp;
- int result;
-
- for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
- {
- if (__glibc_unlikely (inptr[3] > 0x80))
- {
- /* The value is too large. We don't try transliteration here since
- this is not an error because of the lack of possibilities to
- represent the result. This is a genuine bug in the input since
- UCS4 does not allow such values. */
- if (irreversible == NULL)
- /* We are transliterating, don't try to correct anything. */
- return __GCONV_ILLEGAL_INPUT;
-
- if (flags & __GCONV_IGNORE_ERRORS)
- {
- /* Just ignore this character. */
- ++*irreversible;
- continue;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
- return __GCONV_ILLEGAL_INPUT;
- }
-
-# if __BYTE_ORDER == __BIG_ENDIAN
- outptr[3] = inptr[0];
- outptr[2] = inptr[1];
- outptr[1] = inptr[2];
- outptr[0] = inptr[3];
-# else
- outptr[0] = inptr[0];
- outptr[1] = inptr[1];
- outptr[2] = inptr[2];
- outptr[3] = inptr[3];
-# endif
-
- outptr += 4;
- }
-
- *inptrp = inptr;
- *outptrp = outptr;
-
- /* Determine the status. */
- if (*inptrp == inend)
- result = __GCONV_EMPTY_INPUT;
- else if (*inptrp + 4 > inend)
- result = __GCONV_INCOMPLETE_INPUT;
- else
- {
- assert (*outptrp + 4 > outend);
- result = __GCONV_FULL_OUTPUT;
- }
-
- return result;
-}
-#endif
-
static inline int
__attribute ((always_inline))
OK
diff --git a/iconv/loop.c b/iconv/loop.c
index 9d8a7cceb3..b2a1727ad4 100644
--- a/iconv/loop.c
+++ b/iconv/loop.c
@@ -58,12 +58,7 @@
#include <libc-diag.h>
#undef FCTNAME2
-#if _STRING_ARCH_unaligned || !defined DEFINE_UNALIGNED
-# define FCTNAME2(name) name
-#else
-# define FCTNAME2(name) name##_unaligned
-#endif
-#define FCTNAME(name) FCTNAME2(name)
+#define FCTNAME(name) name
/* We need at least one byte for the next round. */
@@ -279,20 +274,9 @@ FCTNAME (LOOPFCT) (struct __gconv_step *step,
}
-/* Include the file a second time to define the function to handle
- unaligned access. */
-#if !defined DEFINE_UNALIGNED && !_STRING_ARCH_unaligned \
- && MIN_NEEDED_INPUT != 1 && MAX_NEEDED_INPUT % MIN_NEEDED_INPUT == 0 \
- && MIN_NEEDED_OUTPUT != 1 && MAX_NEEDED_OUTPUT % MIN_NEEDED_OUTPUT == 0
-# undef unaligned
-
-# define DEFINE_UNALIGNED
-# include "loop.c"
-# undef DEFINE_UNALIGNED
-#else
-# if MAX_NEEDED_INPUT > 1
-# define SINGLE(fct) SINGLE2 (fct)
-# define SINGLE2(fct) fct##_single
+#if MAX_NEEDED_INPUT > 1
+# define SINGLE(fct) SINGLE2 (fct)
+# define SINGLE2(fct) fct##_single
static inline int
__attribute ((always_inline))
SINGLE(LOOPFCT) (struct __gconv_step *step,
@@ -302,37 +286,37 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
size_t *irreversible EXTRA_LOOP_DECLS)
{
mbstate_t *state = step_data->__statep;
-# ifdef LOOP_NEED_FLAGS
+# ifdef LOOP_NEED_FLAGS
int flags = step_data->__flags;
-# endif
-# ifdef LOOP_NEED_DATA
+# endif
+# ifdef LOOP_NEED_DATA
void *data = step->__data;
-# endif
+# endif
int result = __GCONV_OK;
unsigned char bytebuf[MAX_NEEDED_INPUT];
const unsigned char *inptr = *inptrp;
unsigned char *outptr = *outptrp;
size_t inlen;
-# ifdef INIT_PARAMS
+# ifdef INIT_PARAMS
INIT_PARAMS;
-# endif
+# endif
-# ifdef UNPACK_BYTES
+# ifdef UNPACK_BYTES
UNPACK_BYTES
-# else
+# else
/* Add the bytes from the state to the input buffer. */
assert ((state->__count & 7) <= sizeof (state->__value));
for (inlen = 0; inlen < (size_t) (state->__count & 7); ++inlen)
bytebuf[inlen] = state->__value.__wchb[inlen];
-# endif
+# endif
/* Are there enough bytes in the input buffer? */
if (MIN_NEEDED_INPUT > 1
&& __builtin_expect (inptr + (MIN_NEEDED_INPUT - inlen) > inend, 0))
{
*inptrp = inend;
-# ifdef STORE_REST
+# ifdef STORE_REST
/* Building with -O3 GCC emits a `array subscript is above array
bounds' warning. GCC BZ #64739 has been opened for this. */
@@ -347,14 +331,14 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
inend = &bytebuf[inlen];
STORE_REST
-# else
+# else
/* We don't have enough input for another complete input
character. */
size_t inlen_after = inlen + (inend - inptr);
assert (inlen_after <= sizeof (state->__value.__wchb));
for (; inlen < inlen_after; inlen++)
state->__value.__wchb[inlen] = *inptr++;
-# endif
+# endif
return __GCONV_INCOMPLETE_INPUT;
}
@@ -406,11 +390,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
result = __GCONV_OK;
/* Clear the state buffer. */
-# ifdef CLEAR_STATE
+# ifdef CLEAR_STATE
CLEAR_STATE;
-# else
+# else
state->__count &= ~7;
-# endif
+# endif
}
else if (result == __GCONV_INCOMPLETE_INPUT)
{
@@ -419,11 +403,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
assert (inend != &bytebuf[MAX_NEEDED_INPUT]);
*inptrp += inend - bytebuf - (state->__count & 7);
-# ifdef STORE_REST
+# ifdef STORE_REST
inptrp = &inptr;
STORE_REST
-# else
+# else
/* We don't have enough input for another complete input
character. */
assert (inend - inptr > (state->__count & ~7));
@@ -432,14 +416,13 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
for (inlen = 0; inlen < inend - inptr; inlen++)
state->__value.__wchb[inlen] = inptr[inlen];
inptr = inend;
-# endif
+# endif
}
return result;
}
-# undef SINGLE
-# undef SINGLE2
-# endif
+# undef SINGLE
+# undef SINGLE2
# ifdef ONEBYTE_BODY
@@ -471,4 +454,3 @@ gconv_btowc (struct __gconv_step *step, unsigned char c)
#undef LOOP_NEED_STATE
#undef LOOP_NEED_FLAGS
#undef LOOP_NEED_DATA
-#undef unaligned
OK
diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 9423d3fc5a..61cff234ac 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -448,33 +448,6 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
size_t lirreversible = 0;
size_t *lirreversiblep = irreversible ? &lirreversible : NULL;
- /* The following assumes that encodings, which have a variable length
- what might unalign a buffer even though it is an aligned in the
- beginning, either don't have the minimal number of bytes as a divisor
- of the maximum length or have a minimum length of 1. This is true
- for all known and supported encodings.
- We use && instead of || to combine the subexpression for the FROM
- encoding and for the TO encoding, because usually one of them is
- INTERNAL, for which the subexpression evaluates to 1, but INTERNAL
- buffers are always aligned correctly. */
-#define POSSIBLY_UNALIGNED \
- (!_STRING_ARCH_unaligned \
- && (((FROM_LOOP_MIN_NEEDED_FROM != 1 \
- && FROM_LOOP_MAX_NEEDED_FROM % FROM_LOOP_MIN_NEEDED_FROM == 0) \
- && (FROM_LOOP_MIN_NEEDED_TO != 1 \
- && FROM_LOOP_MAX_NEEDED_TO % FROM_LOOP_MIN_NEEDED_TO == 0)) \
- || ((TO_LOOP_MIN_NEEDED_FROM != 1 \
- && TO_LOOP_MAX_NEEDED_FROM % TO_LOOP_MIN_NEEDED_FROM == 0) \
- && (TO_LOOP_MIN_NEEDED_TO != 1 \
- && TO_LOOP_MAX_NEEDED_TO % TO_LOOP_MIN_NEEDED_TO == 0))))
-#if POSSIBLY_UNALIGNED
- int unaligned;
-# define GEN_unaligned(name) GEN_unaligned2 (name)
-# define GEN_unaligned2(name) name##_unaligned
-#else
-# define unaligned 0
-#endif
-
#ifdef PREPARE_LOOP
PREPARE_LOOP
#endif
@@ -514,18 +487,6 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
}
#endif
-#if POSSIBLY_UNALIGNED
- unaligned =
- ((FROM_DIRECTION
- && ((uintptr_t) inptr % FROM_LOOP_MIN_NEEDED_FROM != 0
- || ((data->__flags & __GCONV_IS_LAST)
- && (uintptr_t) outbuf % FROM_LOOP_MIN_NEEDED_TO != 0)))
- || (!FROM_DIRECTION
- && (((data->__flags & __GCONV_IS_LAST)
- && (uintptr_t) outbuf % TO_LOOP_MIN_NEEDED_TO != 0)
- || (uintptr_t) inptr % TO_LOOP_MIN_NEEDED_FROM != 0)));
-#endif
-
while (1)
{
/* Remember the start value for this round. */
@@ -543,34 +504,14 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
SAVE_RESET_STATE (1);
#endif
- if (__glibc_likely (!unaligned))
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- status = FROM_LOOP (step, data, inptrp, inend, &outbuf, outend,
- lirreversiblep EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- status = TO_LOOP (step, data, inptrp, inend, &outbuf, outend,
- lirreversiblep EXTRA_LOOP_ARGS);
- }
-#if POSSIBLY_UNALIGNED
+ if (FROM_DIRECTION)
+ /* Run the conversion loop. */
+ status = FROM_LOOP (step, data, inptrp, inend, &outbuf, outend,
+ lirreversiblep EXTRA_LOOP_ARGS);
else
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- status = GEN_unaligned (FROM_LOOP) (step, data, inptrp, inend,
- &outbuf, outend,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- status = GEN_unaligned (TO_LOOP) (step, data, inptrp, inend,
- &outbuf, outend,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- }
-#endif
+ /* Run the conversion loop. */
+ status = TO_LOOP (step, data, inptrp, inend, &outbuf, outend,
+ lirreversiblep EXTRA_LOOP_ARGS);
/* If we were called as part of an error handling module we
don't do anything else here. */
@@ -635,41 +576,18 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
SAVE_RESET_STATE (0);
#endif
- if (__glibc_likely (!unaligned))
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- nstatus = FROM_LOOP (step, data, inptrp, inend,
- &outbuf, outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- nstatus = TO_LOOP (step, data, inptrp, inend,
- &outbuf, outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- }
-#if POSSIBLY_UNALIGNED
+ if (FROM_DIRECTION)
+ /* Run the conversion loop. */
+ nstatus = FROM_LOOP (step, data, inptrp, inend,
+ &outbuf, outerr,
+ lirreversiblep
+ EXTRA_LOOP_ARGS);
else
- {
- if (FROM_DIRECTION)
- /* Run the conversion loop. */
- nstatus = GEN_unaligned (FROM_LOOP) (step, data,
- inptrp, inend,
- &outbuf,
- outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- else
- /* Run the conversion loop. */
- nstatus = GEN_unaligned (TO_LOOP) (step, data,
- inptrp, inend,
- &outbuf, outerr,
- lirreversiblep
- EXTRA_LOOP_ARGS);
- }
-#endif
+ /* Run the conversion loop. */
+ nstatus = TO_LOOP (step, data, inptrp, inend,
+ &outbuf, outerr,
+ lirreversiblep
+ EXTRA_LOOP_ARGS);
/* We must run out of output buffer space in this
rerun. */
OK
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 7/7] string: Remove string_private.h
2023-02-13 13:55 [PATCH 0/7] Remove _STRING_ARCH_unaligned Adhemerval Zanella
` (5 preceding siblings ...)
2023-02-13 13:55 ` [PATCH 6/7] iconv: Remove _STRING_ARCH_unaligned usage Adhemerval Zanella
@ 2023-02-13 13:55 ` Adhemerval Zanella
2023-02-15 19:04 ` Wilco Dijkstra
6 siblings, 1 reply; 23+ messages in thread
From: Adhemerval Zanella @ 2023-02-13 13:55 UTC (permalink / raw)
To: libc-alpha, Wilco Dijkstra
Now that _STRING_ARCH_unaligned is not used anymore.
---
include/string.h | 3 ---
sysdeps/aarch64/string_private.h | 20 --------------------
sysdeps/generic/string_private.h | 21 ---------------------
sysdeps/m68k/m680x0/m68020/string_private.h | 21 ---------------------
sysdeps/s390/string_private.h | 20 --------------------
sysdeps/x86/string_private.h | 20 --------------------
6 files changed, 105 deletions(-)
delete mode 100644 sysdeps/aarch64/string_private.h
delete mode 100644 sysdeps/generic/string_private.h
delete mode 100644 sysdeps/m68k/m680x0/m68020/string_private.h
delete mode 100644 sysdeps/s390/string_private.h
delete mode 100644 sysdeps/x86/string_private.h
diff --git a/include/string.h b/include/string.h
index a9120ff37c..673cfd7272 100644
--- a/include/string.h
+++ b/include/string.h
@@ -55,9 +55,6 @@ extern char *__strerror_l (int __errnum, locale_t __loc);
extern const char *__sigdescr_np (int __errnum);
libc_hidden_proto (__sigdescr_np)
-
-/* Get _STRING_ARCH_unaligned. */
-#include <string_private.h>
#endif
#include <string/string.h>
diff --git a/sysdeps/aarch64/string_private.h b/sysdeps/aarch64/string_private.h
deleted file mode 100644
index 8fdfcdf44b..0000000000
--- a/sysdeps/aarch64/string_private.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Define _STRING_ARCH_unaligned. AArch64 version.
- Copyright (C) 2016-2023 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
- <https://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
deleted file mode 100644
index a1a62b3a46..0000000000
--- a/sysdeps/generic/string_private.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Define _STRING_ARCH_unaligned. Generic version.
- Copyright (C) 2016-2023 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
- <https://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/string_private.h b/sysdeps/m68k/m680x0/m68020/string_private.h
deleted file mode 100644
index 04c4800244..0000000000
--- a/sysdeps/m68k/m680x0/m68020/string_private.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Define _STRING_ARCH_unaligned. m680x0 version, x >= 2.
- Copyright (C) 2016-2023 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
- <https://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/string_private.h b/sysdeps/s390/string_private.h
deleted file mode 100644
index 3591f83878..0000000000
--- a/sysdeps/s390/string_private.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Define _STRING_ARCH_unaligned. S/390 version.
- Copyright (C) 2016-2023 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
- <https://www.gnu.org/licenses/>. */
-
-/* The s390 processors can access unaligned multi-byte variables. */
-#define _STRING_ARCH_unaligned 1
diff --git a/sysdeps/x86/string_private.h b/sysdeps/x86/string_private.h
deleted file mode 100644
index c6a9d9bef9..0000000000
--- a/sysdeps/x86/string_private.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Define _STRING_ARCH_unaligned. i486/x86-64 version.
- Copyright (C) 2016-2023 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
- <https://www.gnu.org/licenses/>. */
-
-/* The ix86 processors can access unaligned multi-byte variables. */
-#define _STRING_ARCH_unaligned 1
--
2.34.1
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 7/7] string: Remove string_private.h
2023-02-13 13:55 ` [PATCH 7/7] string: Remove string_private.h Adhemerval Zanella
@ 2023-02-15 19:04 ` Wilco Dijkstra
0 siblings, 0 replies; 23+ messages in thread
From: Wilco Dijkstra @ 2023-02-15 19:04 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Hi Adhemerval,
LGTM.
Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
diff --git a/include/string.h b/include/string.h
index a9120ff37c..673cfd7272 100644
--- a/include/string.h
+++ b/include/string.h
@@ -55,9 +55,6 @@ extern char *__strerror_l (int __errnum, locale_t __loc);
extern const char *__sigdescr_np (int __errnum);
libc_hidden_proto (__sigdescr_np)
-
-/* Get _STRING_ARCH_unaligned. */
-#include <string_private.h>
#endif
#include <string/string.h>
diff --git a/sysdeps/aarch64/string_private.h b/sysdeps/aarch64/string_private.h
deleted file mode 100644
index 8fdfcdf44b..0000000000
--- a/sysdeps/aarch64/string_private.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Define _STRING_ARCH_unaligned. AArch64 version.
- Copyright (C) 2016-2023 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
- <https://www.gnu.org/licenses/>. */
-
-/* AArch64 implementations support efficient unaligned access. */
-#define _STRING_ARCH_unaligned 1
OK
diff --git a/sysdeps/generic/string_private.h b/sysdeps/generic/string_private.h
deleted file mode 100644
index a1a62b3a46..0000000000
--- a/sysdeps/generic/string_private.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Define _STRING_ARCH_unaligned. Generic version.
- Copyright (C) 2016-2023 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
- <https://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/string_private.h b/sysdeps/m68k/m680x0/m68020/string_private.h
deleted file mode 100644
index 04c4800244..0000000000
--- a/sysdeps/m68k/m680x0/m68020/string_private.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Define _STRING_ARCH_unaligned. m680x0 version, x >= 2.
- Copyright (C) 2016-2023 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
- <https://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/string_private.h b/sysdeps/s390/string_private.h
deleted file mode 100644
index 3591f83878..0000000000
--- a/sysdeps/s390/string_private.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Define _STRING_ARCH_unaligned. S/390 version.
- Copyright (C) 2016-2023 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
- <https://www.gnu.org/licenses/>. */
-
-/* The s390 processors can access unaligned multi-byte variables. */
-#define _STRING_ARCH_unaligned 1
diff --git a/sysdeps/x86/string_private.h b/sysdeps/x86/string_private.h
deleted file mode 100644
index c6a9d9bef9..0000000000
--- a/sysdeps/x86/string_private.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* Define _STRING_ARCH_unaligned. i486/x86-64 version.
- Copyright (C) 2016-2023 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
- <https://www.gnu.org/licenses/>. */
-
-/* The ix86 processors can access unaligned multi-byte variables. */
-#define _STRING_ARCH_unaligned 1
OK
^ permalink raw reply [flat|nested] 23+ messages in thread