Re: [PATCH][Testsuite] Use user defined memmove in gcc.c-torture/execute/builtins/memops-asm-lib.c

[Tom de Vries]

On 06/23/2017 11:19 AM, Renlin Li wrote:

+__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;
+}
+}
+


The memops-asm.c testcase fails for nvptx because of a mismatch between 
function definition and function declaration for my_memmove in the 
generated nvptx code.


man memmove show us that memmove returns a 'void *', not 'void':
...
   void *memmove(void *dest, const void *src, size_t n);
...

and that it returns its first argument (well, the formulation could be 
improved):

...
RETURN VALUE
   The memmove() function returns a pointer to dest.
...


Fixed in attached patch.

Committed.

Thanks,
- Tom
Fix my_memmove in gcc.c-torture/execute/builtins/memops-asm-lib.c

2017-08-06  Tom de Vries  

	* gcc.c-torture/execute/builtins/memops-asm-lib.c (my_memmove):  Fix return
	type.  Add missing return.

---
 gcc/testsuite/gcc.c-torture/execute/builtins/memops-asm-lib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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 25d4a40..3baf7a6 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
@@ -38,7 +38,7 @@ my_bcopy (const void *s, void *d, size_t n)
 }
 
 __attribute__ ((used))
-void
+void *
 my_memmove (void *d, const void *s, size_t n)
 {
   char *dst = (char *) d;
@@ -53,6 +53,8 @@ my_memmove (void *d, const void *s, size_t n)
   while (n--)
 	*--dst = *--src;
 }
+
+  return d;
 }
 
 /* LTO code is at the present to able to track that asm alias my_bcopy on builtin

Re: [PATCH][Testsuite] Use user defined memmove in gcc.c-torture/execute/builtins/memops-asm-lib.c

[Jeff Law]

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  
> Szabolcs Nagy  
> 
> * 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

Re: [PATCH][Testsuite] Use user defined memmove in gcc.c-torture/execute/builtins/memops-asm-lib.c

[Renlin Li]

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  
Szabolcs Nagy  

* gcc.c-torture/execute/builtins/memops-asm-lib.c (my_memmove): New.
* gcc.c-torture/execute/builtins/memops-asm.c (memmove): Declare
memmove.

Re: [PATCH][Testsuite] Use user defined memmove in gcc.c-torture/execute/builtins/memops-asm-lib.c

[Martin Sebor]

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  
Szabolcs Nagy  

* gcc.c-torture/execute/builtins/memops-asm-lib.c (my_memmove): New.
* gcc.c-torture/execute/builtins/memops-asm.c (memmove): Declare
memmove.