From: Noah Goldstein <goldstein.w.n@gmail.com>
To: Gabriel Ravier <gabravier@gmail.com>
Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
libc-alpha@sourceware.org, Paul Eggert <eggert@cs.ucla.edu>,
Florian Weimer <fweimer@redhat.com>
Subject: Re: [PATCH v8 1/7] string: Add internal memswap implementation
Date: Fri, 27 Oct 2023 20:30:28 -0500 [thread overview]
Message-ID: <CAFUsyfLvj4NX=hCOXsnmU5HNxyRShqDSLiK3ExQ1E7y-QfTKsw@mail.gmail.com> (raw)
In-Reply-To: <1b4a07e3-cf6d-4f71-9c07-a9079274e9fb@gmail.com>
On Fri, Oct 27, 2023 at 5:47 PM Gabriel Ravier <gabravier@gmail.com> wrote:
>
> On 10/27/23 21:23, Adhemerval Zanella Netto wrote:
> >
> > 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?
>
> My interpretation was that this is the less commonly used meaning of "in
> order" as meaning "appropriate in the current circumstances", i.e. that
> tests that invoke memswap with unaligned operands seem like they would
> be useful.
^
>
next prev parent reply other threads:[~2023-10-28 1:30 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-03 12:22 [PATCH v8 0/7] Use introsort for qsort Adhemerval Zanella
2023-10-03 12:22 ` [PATCH v8 1/7] string: Add internal memswap implementation Adhemerval Zanella
2023-10-03 17:42 ` Noah Goldstein
2023-10-27 20:23 ` Adhemerval Zanella Netto
2023-10-27 22:47 ` Gabriel Ravier
2023-10-28 1:30 ` Noah Goldstein [this message]
2023-10-30 19:02 ` Noah Goldstein
2023-10-03 12:22 ` [PATCH v8 2/7] stdlib: Optimization qsort{_r} swap implementation Adhemerval Zanella
2023-10-30 19:02 ` Noah Goldstein
2023-10-03 12:22 ` [PATCH v8 3/7] stdlib: Move insertion sort out qsort Adhemerval Zanella
2023-10-30 19:03 ` Noah Goldstein
2023-10-03 12:22 ` [PATCH v8 4/7] stdlib: qsort: Move some macros to inline function Adhemerval Zanella
2023-10-30 19:03 ` Noah Goldstein
2023-10-03 12:22 ` [PATCH v8 5/7] stdlib: Implement introsort for qsort (BZ 19305) Adhemerval Zanella
2023-10-30 19:03 ` Noah Goldstein
2023-10-03 12:22 ` [PATCH v8 6/7] stdlib: Remove use of mergesort on qsort (BZ 21719) Adhemerval Zanella
2023-10-30 19:04 ` Noah Goldstein
2023-10-03 12:22 ` [PATCH v8 7/7] stdlib: Add more qsort{_r} coverage Adhemerval Zanella
2023-10-30 19:04 ` Noah Goldstein
2023-10-03 18:13 ` [PATCH v8 0/7] Use introsort for qsort Noah Goldstein
2023-10-27 20:24 ` Adhemerval Zanella Netto
2023-10-28 1:31 ` Noah Goldstein
-- strict thread matches above, loose matches on Subject: below --
2023-10-02 19:33 Adhemerval Zanella
2023-10-02 19:33 ` [PATCH v8 1/7] string: Add internal memswap implementation Adhemerval Zanella
2023-10-03 5:43 ` Noah Goldstein
2023-10-03 11:00 ` Adhemerval Zanella Netto
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFUsyfLvj4NX=hCOXsnmU5HNxyRShqDSLiK3ExQ1E7y-QfTKsw@mail.gmail.com' \
--to=goldstein.w.n@gmail.com \
--cc=adhemerval.zanella@linaro.org \
--cc=eggert@cs.ucla.edu \
--cc=fweimer@redhat.com \
--cc=gabravier@gmail.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).