| Message ID | 594CDD3A.9090902@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 06/23/2017 03:19 AM, Renlin Li wrote: > Hi all, > > After the change r249278. bcopy is folded into memmove. And in newlib > aarch64 > memmove implementation, it will call memcpy in certain conditions. > The memcpy defined in memops-asm-lib.c will abort when the test is running. > > In this case, I defined a user memmove function which by pass the > library one. > So that memcpy won't be called accidentally. > > Okay to commit? Having memmove call memcpy when there is no overlap seems like a valid transformation. I don't know which test specifically fails so the question on my mind is whether it perhaps is overly restrictive in assuming that this transformation must never take place. Other than that, although I can't really approve patches, this one looks okay to me. Thanks for getting to the bottom of the failure and fixing it! Martin > > gcc/testsuite/ChangeLog: > > 2017-06-22 Renlin Li <renlin.li@arm.com> > Szabolcs Nagy <szabolcs.nagy@arm.com> > > * gcc.c-torture/execute/builtins/memops-asm-lib.c (my_memmove): New. > * gcc.c-torture/execute/builtins/memops-asm.c (memmove): Declare > memmove.
Hi Martin, On 23/06/17 16:27, Martin Sebor wrote: > On 06/23/2017 03:19 AM, Renlin Li wrote: >> Hi all, >> >> After the change r249278. bcopy is folded into memmove. And in newlib >> aarch64 >> memmove implementation, it will call memcpy in certain conditions. >> The memcpy defined in memops-asm-lib.c will abort when the test is running. >> >> In this case, I defined a user memmove function which by pass the >> library one. >> So that memcpy won't be called accidentally. >> >> Okay to commit? > > Having memmove call memcpy when there is no overlap seems like > a valid transformation. I don't know which test specifically > fails so the question on my mind is whether it perhaps is overly > restrictive in assuming that this transformation must never take > place. Other than that, although I can't really approve patches, > this one looks okay to me. Thanks for getting to the bottom of > the failure and fixing it! Sorry I didn't mention the regressions. It only happens with aarch64 baremetal targets because of the newlib memmove implementation. FAIL: gcc.c-torture/execute/builtins/memops-asm.c execution, -O0 FAIL: gcc.c-torture/execute/builtins/memops-asm.c execution, -O1 FAIL: gcc.c-torture/execute/builtins/memops-asm.c execution, -O2 FAIL: gcc.c-torture/execute/builtins/memops-asm.c execution, -O3 -g FAIL: gcc.c-torture/execute/builtins/memops-asm.c execution, -Og -g FAIL: gcc.c-torture/execute/builtins/memops-asm.c execution, -Os I think the purpose of the test is to check, the original function is not directly called from the main_test function. Instead, those calls are redirected to "my_" version. It will abort otherwise. I CCed Richard Sandiford as he is the original contributor of the test case. Before r249278, bcopy has a corresponding my_bcopy function which is actually got called. Regards, Renlin > > Martin > >> >> gcc/testsuite/ChangeLog: >> >> 2017-06-22 Renlin Li <renlin.li@arm.com> >> Szabolcs Nagy <szabolcs.nagy@arm.com> >> >> * gcc.c-torture/execute/builtins/memops-asm-lib.c (my_memmove): New. >> * gcc.c-torture/execute/builtins/memops-asm.c (memmove): Declare >> memmove. >
On 06/23/2017 03:19 AM, Renlin Li wrote: > Hi all, > > After the change r249278. bcopy is folded into memmove. And in newlib > aarch64 > memmove implementation, it will call memcpy in certain conditions. > The memcpy defined in memops-asm-lib.c will abort when the test is running. > > In this case, I defined a user memmove function which by pass the > library one. > So that memcpy won't be called accidentally. > > Okay to commit? > > gcc/testsuite/ChangeLog: > > 2017-06-22 Renlin Li <renlin.li@arm.com> > Szabolcs Nagy <szabolcs.nagy@arm.com> > > * gcc.c-torture/execute/builtins/memops-asm-lib.c (my_memmove): New. > * gcc.c-torture/execute/builtins/memops-asm.c (memmove): Declare > memmove. OK. jeff
diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c b/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c index 0000529..25d4a40 100644 --- a/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c +++ b/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c @@ -37,6 +37,24 @@ my_bcopy (const void *s, void *d, size_t n) } } +__attribute__ ((used)) +void +my_memmove (void *d, const void *s, size_t n) +{ + char *dst = (char *) d; + const char *src = (const char *) s; + if (src >= dst) + while (n--) + *dst++ = *src++; + else + { + dst += n; + src += n; + while (n--) + *--dst = *--src; + } +} + /* LTO code is at the present to able to track that asm alias my_bcopy on builtin actually refers to this function. See PR47181. */ __attribute__ ((used)) diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm.c b/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm.c index ed2b06c..44e336c 100644 --- a/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm.c +++ b/gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm.c @@ -12,6 +12,8 @@ extern void *memcpy (void *, const void *, size_t) __asm (ASMNAME ("my_memcpy")); extern void bcopy (const void *, void *, size_t) __asm (ASMNAME ("my_bcopy")); +extern void *memmove (void *, const void *, size_t) + __asm (ASMNAME ("my_memmove")); extern void *memset (void *, int, size_t) __asm (ASMNAME ("my_memset")); extern void bzero (void *, size_t)