Re: [PATCH v8 1/7] string: Add internal memswap implementation

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 03/10/23 14:42, Noah Goldstein wrote:
> On Tue, Oct 3, 2023 at 5:22 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> The prototype is:
>>
>>   void __memswap (void *restrict p1, void *restrict p2, size_t n)
>>
>> The function swaps the content of two memory blocks P1 and P2 of
>> len N.  Memory overlap is NOT handled.
>>
>> It will be used on qsort optimization.
>>
>> Checked on x86_64-linux-gnu and aarch64-linux-gnu.
>> ---
>>  string/Makefile           |  12 +++
>>  string/test-memswap.c     | 192 ++++++++++++++++++++++++++++++++++++++
>>  sysdeps/generic/memswap.h |  41 ++++++++
>>  3 files changed, 245 insertions(+)
>>  create mode 100644 string/test-memswap.c
>>  create mode 100644 sysdeps/generic/memswap.h
>>
>> diff --git a/string/Makefile b/string/Makefile
>> index 8cdfd5b000..fb101db778 100644
>> --- a/string/Makefile
>> +++ b/string/Makefile
>> @@ -209,6 +209,18 @@ tests := \
>>    tst-xbzero-opt \
>>  # tests
>>
>> +tests-static-internal := \
>> +  test-memswap \
>> +# tests-static-internal
>> +
>> +tests-internal := \
>> +  $(tests-static-internal) \
>> +  # tests-internal
>> +
>> +tests-static := \
>> +  $(tests-static-internal) \
>> +  # tests-static
>> +
>>  # Both tests require the .mo translation files generated by msgfmt.
>>  tests-translation := \
>>    tst-strerror \
>> diff --git a/string/test-memswap.c b/string/test-memswap.c
>> new file mode 100644
>> index 0000000000..162beb91e3
>> --- /dev/null
>> +++ b/string/test-memswap.c
>> @@ -0,0 +1,192 @@
>> +/* Test and measure memcpy functions.
>> +   Copyright (C) 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/>.  */
>> +
>> +#include <string.h>
>> +#include <support/check.h>
>> +#include <memswap.h>
>> +
>> +#define TEST_MAIN
>> +#define BUF1PAGES 3
>> +#include "test-string.h"
>> +
>> +static unsigned char *ref1;
>> +static unsigned char *ref2;
>> +
>> +static void
>> +do_one_test (unsigned char *p1, unsigned char *ref1, unsigned char *p2,
>> +            unsigned char *ref2, size_t len)
>> +{
>> +  __memswap (p1, p2, len);
>> +
>> +  TEST_COMPARE_BLOB (p1, len, ref2, len);
>> +  TEST_COMPARE_BLOB (p2, len, ref1, len);
>> +}
>> +
>> +static inline void
>> +do_test (size_t align1, size_t align2, size_t len)
>> +{
>> +  align1 &= page_size;
>> +  if (align1 + len >= page_size)
>> +    return;
>> +
>> +  align2 &= page_size;
>> +  if (align2 + len >= page_size)
>> +    return;
>> +
>> +  unsigned char *p1 = buf1 + align1;
>> +  unsigned char *p2 = buf2 + align2;
>> +  for (size_t repeats = 0; repeats < 2; ++repeats)
>> +    {
>> +      size_t i, j;
>> +      for (i = 0, j = 1; i < len; i++, j += 23)
>> +       {
>> +         ref1[i] = p1[i] = j;
>> +         ref2[i] = p2[i] = UCHAR_MAX - j;
>> +       }
>> +
>> +      do_one_test (p1, ref1, p2, ref2, len);
>> +    }
>> +}
>> +
>> +static void
>> +do_random_tests (void)
>> +{
>> +  for (size_t n = 0; n < ITERATIONS; n++)
>> +    {
>> +      size_t len, size, size1, size2, align1, align2;
>> +
>> +      if (n == 0)
>> +        {
>> +          len = getpagesize ();
>> +          size = len + 512;
>> +          size1 = size;
>> +          size2 = size;
>> +          align1 = 512;
>> +          align2 = 512;
>> +        }
>> +      else
>> +        {
>> +          if ((random () & 255) == 0)
>> +            size = 65536;
>> +          else
>> +            size = 768;
>> +          if (size > page_size)
>> +            size = page_size;
>> +          size1 = size;
>> +          size2 = size;
>> +          size_t i = random ();
>> +          if (i & 3)
>> +            size -= 256;
>> +          if (i & 1)
>> +            size1 -= 256;
>> +          if (i & 2)
>> +            size2 -= 256;
> Some tests with misaligned size seem in order.

I am not sure I understood what is wrong here, by 'in order' do you mean
exactly what?