From: Noah Goldstein <goldstein.w.n@gmail.com>
To: Stafford Horne <shorne@gmail.com>
Cc: GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH v4 1/5] string: Make tests birdirectional test-memcpy.c
Date: Tue, 7 Dec 2021 16:13:54 -0600 [thread overview]
Message-ID: <CAFUsyf+xMXtJLNvKJQAKGB9Cy8W7qLFMK=v7-Qjpd-XFfzA5qw@mail.gmail.com> (raw)
In-Reply-To: <Ya/bLWdrb45OnBR6@antec>
On Tue, Dec 7, 2021 at 4:07 PM Stafford Horne <shorne@gmail.com> wrote:
>
> On Tue, Dec 07, 2021 at 03:36:48PM -0600, Noah Goldstein wrote:
> > On Tue, Dec 7, 2021 at 3:10 PM Stafford Horne <shorne@gmail.com> wrote:
> > >
> > > On Sat, Nov 06, 2021 at 01:33:18PM -0500, Noah Goldstein via Libc-alpha wrote:
> > > > This commit updates the memcpy tests to test both dst > src and dst <
> > > > src. This is because there is logic in the code based on the
> > > > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > > > ---
> > > > string/test-memcpy.c | 167 +++++++++++++++++++++++++++++++++++-------
> > > > string/test-memmove.c | 75 ++++++++++++++++++-
> > > > 2 files changed, 214 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > > [..]
> > > > static void
> > > > -do_test1 (size_t size)
> > > > +do_test1 (size_t align1, size_t align2, size_t size)
> > > > {
> > > > void *large_buf;
> > > > - large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> > > > - MAP_PRIVATE | MAP_ANON, -1, 0);
> > > > + size_t mmap_size, region_size;
> > > > +
> > > > + align1 &= (page_size - 1);
> > > > + if (align1 == 0)
> > > > + align1 = page_size;
> > > > +
> > > > + align2 &= (page_size - 1);
> > > > + if (align2 == 0)
> > > > + align2 = page_size;
> > > > +
> > > > + region_size = (size + page_size - 1) & (~(page_size - 1));
> > > > +
> > > > + mmap_size = region_size * 2 + 3 * page_size;
> > > > + large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> > > > + MAP_PRIVATE | MAP_ANON, -1, 0);
> > > > if (large_buf == MAP_FAILED)
> > > > {
> > > > - puts ("Failed to allocat large_buf, skipping do_test1");
> > > > + puts ("Failed to allocate large_buf, skipping do_test1");
> > > > return;
> > > > }
> > > > -
> > > > - if (mprotect (large_buf + size, page_size, PROT_NONE))
> > > > + if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
> > > > error (EXIT_FAILURE, errno, "mprotect failed");
> > > >
> > > > - size_t arrary_size = size / sizeof (uint32_t);
> > > > - uint32_t *dest = large_buf;
> > > > - uint32_t *src = large_buf + size + page_size;
> > > > + size_t array_size = size / sizeof (uint32_t);
> > > > + uint32_t *dest = large_buf + align1;
> > > > + uint32_t *src = large_buf + region_size + 2 * page_size + align2;
> > >
> > > Hello, this causes Bus errors on the new OpenRISC port I am working on.
> >
> > Bugzilla for this issue is here:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=28572
> >
> > There is a patch attached to the bugzilla and a patch proposed
> > by Adhemerval Zanella that both fix the issue.
>
> Thanks, That patch should work.
>
> -Stafford
Just posted it.
>
> > >
> > > > size_t i;
> > > > size_t repeats;
> > > > for(repeats = 0; repeats < 2; repeats++)
> > > > {
> > > > - for (i = 0; i < arrary_size; i++)
> > > > + for (i = 0; i < array_size; i++)
> > > > src[i] = (uint32_t) i;
> > >
> > > The bus errors happen here caused when align2 is 1 or 2. OpenRISC (and maybe
> > > other architectures?) do not support unaligned copies of words.
> > >
> > > I fixed this by limiting the align1/align2 to 4 but I am not sure if that is
> > > what you are trying to copy here.
> > >
> > > Maybe we need to change how we setup the src array.
> > >
> > > > -
> > > > FOR_EACH_IMPL (impl, 0)
> > > > {
> > > > - printf ("\t\tRunning: %s\n", impl->name);
> > > > memset (dest, -1, size);
> > > > CALL (impl, (char *) dest, (char *) src, size);
> > > > - for (i = 0; i < arrary_size; i++)
> > > > + for (i = 0; i < array_size; i++)
> > > > if (dest[i] != src[i])
> > > > {
> > > > error (0, 0,
> > > > "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> > > > impl->name, dest, src, i);
> > > > ret = 1;
> > > > - munmap ((void *) large_buf, size * 2 + page_size);
> > > > + munmap ((void *) large_buf, mmap_size);
> > > > return;
> > > > }
> > > > }
> > > > - dest = src;
> > > > - src = large_buf;
> > > > + dest = large_buf + region_size + 2 * page_size + align1;
> > > > + src = large_buf + align2;
> > > > + }
> > > > + munmap ((void *) large_buf, mmap_size);
> > > > +}
> > > > +
> > > [..]
> > > > @@ -306,8 +341,88 @@ test_main (void)
> > > >
> > > > do_random_tests ();
> > > >
> > > > - do_test1 (0x100000);
> > > > - do_test1 (0x2000000);
> > > > + do_test1 (0, 0, 0x100000);
> > > > + do_test1 (0, 0, 0x2000000);
> > > > +
> > > > + for (i = 4096; i < 32768; i += 4096)
> > > > + {
> > > > + for (j = 1; j <= 1024; j <<= 1)
> > > > + {
> > > > + do_test1 (0, j, i);
> > > > + do_test1 (4095, j, i);
> > > > + do_test1 (4096 - j, 0, i);
> > > > +
> > > > + do_test1 (0, j - 1, i);
> > > > + do_test1 (4095, j - 1, i);
> > > > + do_test1 (4096 - j - 1, 0, i);
> > > > +
> > > > + do_test1 (0, j + 1, i);
> > > > + do_test1 (4095, j + 1, i);
> > > > + do_test1 (4096 - j, 1, i);
> > >
> > > These +1, -1's cause non-aligned word access.
> > >
> > > > + }
> > > > + }
> > > > +
> > >
> > > -Stafford
prev parent reply other threads:[~2021-12-07 22:14 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-01 5:49 [PATCH v1 " Noah Goldstein
2021-11-01 5:49 ` [PATCH v1 2/5] benchtests: Add additional cases to bench-memcpy.c and bench-memmove.c Noah Goldstein
2021-11-06 2:27 ` H.J. Lu
2021-11-01 5:49 ` [PATCH v1 3/5] benchtests: Add partial overlap case in bench-memmove-walk.c Noah Goldstein
2021-11-06 2:28 ` H.J. Lu
2021-11-01 5:49 ` [PATCH v1 4/5] x86: Optimize memmove-vec-unaligned-erms.S Noah Goldstein
2021-11-01 5:52 ` Noah Goldstein
2021-11-06 2:29 ` H.J. Lu
2021-11-01 5:49 ` [PATCH v1 5/5] x86: Double size of ERMS rep_movsb_threshold in dl-cacheinfo.h Noah Goldstein
2021-11-06 2:31 ` H.J. Lu
2021-11-06 4:39 ` Noah Goldstein
2021-11-06 12:04 ` H.J. Lu
2021-11-06 17:38 ` Noah Goldstein
2021-11-06 2:27 ` [PATCH v1 1/5] string: Make tests birdirectional test-memcpy.c H.J. Lu
2021-11-06 4:39 ` [PATCH v2 " Noah Goldstein
2021-11-06 4:39 ` [PATCH v2 2/5] benchtests: Add additional cases to bench-memcpy.c and bench-memmove.c Noah Goldstein
2021-11-06 4:39 ` [PATCH v2 3/5] benchtests: Add partial overlap case in bench-memmove-walk.c Noah Goldstein
2021-11-06 4:39 ` [PATCH v2 4/5] x86: Optimize memmove-vec-unaligned-erms.S Noah Goldstein
2021-11-06 4:39 ` [PATCH v2 5/5] x86: Double size of ERMS rep_movsb_threshold in dl-cacheinfo.h Noah Goldstein
2021-11-06 17:37 ` [PATCH v3 1/5] string: Make tests birdirectional test-memcpy.c Noah Goldstein
2021-11-06 17:37 ` [PATCH v3 2/5] benchtests: Add additional cases to bench-memcpy.c and bench-memmove.c Noah Goldstein
2021-11-06 17:37 ` [PATCH v3 3/5] benchtests: Add partial overlap case in bench-memmove-walk.c Noah Goldstein
2021-11-06 17:37 ` [PATCH v3 4/5] x86: Optimize memmove-vec-unaligned-erms.S Noah Goldstein
2021-11-06 17:37 ` [PATCH v3 5/5] x86: Double size of ERMS rep_movsb_threshold in dl-cacheinfo.h Noah Goldstein
2021-11-06 17:56 ` H.J. Lu
2021-11-06 18:11 ` Noah Goldstein
2021-11-06 18:21 ` H.J. Lu
2021-11-06 18:34 ` Noah Goldstein
2021-11-06 18:33 ` [PATCH v4 1/5] string: Make tests birdirectional test-memcpy.c Noah Goldstein
2021-11-06 18:33 ` [PATCH v4 2/5] benchtests: Add additional cases to bench-memcpy.c and bench-memmove.c Noah Goldstein
2021-11-06 19:12 ` H.J. Lu
2021-11-06 18:33 ` [PATCH v4 3/5] benchtests: Add partial overlap case in bench-memmove-walk.c Noah Goldstein
2021-11-06 19:11 ` H.J. Lu
2021-11-06 18:33 ` [PATCH v4 4/5] x86: Optimize memmove-vec-unaligned-erms.S Noah Goldstein
2021-11-06 19:11 ` H.J. Lu
2022-04-23 1:41 ` Sunil Pandey
2021-11-06 18:33 ` [PATCH v4 5/5] x86: Double size of ERMS rep_movsb_threshold in dl-cacheinfo.h Noah Goldstein
2021-11-06 19:10 ` H.J. Lu
2022-04-23 1:42 ` Sunil Pandey
2021-11-06 19:12 ` [PATCH v4 1/5] string: Make tests birdirectional test-memcpy.c H.J. Lu
2021-11-06 21:20 ` Noah Goldstein
2021-11-07 13:53 ` H.J. Lu
2021-12-07 21:10 ` Stafford Horne
2021-12-07 21:36 ` Noah Goldstein
2021-12-07 22:07 ` Stafford Horne
2021-12-07 22:13 ` Noah Goldstein [this message]
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='CAFUsyf+xMXtJLNvKJQAKGB9Cy8W7qLFMK=v7-Qjpd-XFfzA5qw@mail.gmail.com' \
--to=goldstein.w.n@gmail.com \
--cc=libc-alpha@sourceware.org \
--cc=shorne@gmail.com \
/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).