diff mbox

Fix Wundef warning for MEMCPY_OK_FOR_FWD_MEMMOVE

Message ID 20140626185412.GA1826@spoyarek.pnq.redhat.com
State Superseded
Headers show

Commit Message

Siddhesh Poyarekar June 26, 2014, 6:54 p.m. UTC
Define MEMCPY_OK_FOR_FWD_MEMMOVE to 0 in x86_64 memmove.c before
including the generic string/memmove.c to get rid of this warning.

Tested to verify on x86_64 that the generated code is identical.

Siddhesh

	* sysdeps/x86_64/memmove.c: Define MEMCPY_OK_FOR_FWD_MEMMOVE
	to 0.
	* sysdeps/x86_64/multiarch/memmove.c: Likewise.

---
 sysdeps/x86_64/memmove.c           | 1 +
 sysdeps/x86_64/multiarch/memmove.c | 1 +
 2 files changed, 2 insertions(+)

Comments

Roland McGrath June 26, 2014, 7:04 p.m. UTC | #1
That only helps x86_64 and not anybody else using that file (whether
directly by sysdeps selection or by #include).  I think memcopy.h should
be required to define that one way or the other.  Then we can have a
default in sysdeps/generic/memcopy.h.
Siddhesh Poyarekar June 27, 2014, 9:21 a.m. UTC | #2
On Thu, Jun 26, 2014 at 12:04:23PM -0700, Roland McGrath wrote:
> That only helps x86_64 and not anybody else using that file (whether
> directly by sysdeps selection or by #include).  I think memcopy.h should
> be required to define that one way or the other.  Then we can have a
> default in sysdeps/generic/memcopy.h.

Wouldn't that not catch cases where one forgets to define the macro
and inherits the default?

Siddhesh
Will Newton June 27, 2014, 9:28 a.m. UTC | #3
On 27 June 2014 10:21, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
> On Thu, Jun 26, 2014 at 12:04:23PM -0700, Roland McGrath wrote:
>> That only helps x86_64 and not anybody else using that file (whether
>> directly by sysdeps selection or by #include).  I think memcopy.h should
>> be required to define that one way or the other.  Then we can have a
>> default in sysdeps/generic/memcopy.h.
>
> Wouldn't that not catch cases where one forgets to define the macro
> and inherits the default?

It's an optimization hint so it should be ok to inherit the default of
"use the slow copy loop".
Siddhesh Poyarekar June 27, 2014, 5:30 p.m. UTC | #4
... and I forgot to add Chris to cc to bring this to his attention.

Siddhesh

On Fri, Jun 27, 2014 at 10:43:26PM +0530, Siddhesh Poyarekar wrote:
> I've moved the macro definition to memcopy.h with tile overriding the
> definition in its memcopy.h.  This also allows us to get rid of the
> tile-specific memmove.c.  The tile bit is untested, so I'll need Chris
> to verify that it doesn't cause any problems.  The code is unchanged
> on x86_64.
> 
> Siddhesh
> 
> 	* sysdeps/generic/memcopy.h: Define MEMCPY_OK_FOR_FWD_MEMMOVE.
> 	* sysdeps/tile/memcopy.h: Redefine MEMCPY_OK_FOR_FWD_MEMMOVE.
> 	* sysdeps/tile/tilegx/memmove.c: Remove file.
> 
> diff --git a/sysdeps/generic/memcopy.h b/sysdeps/generic/memcopy.h
> index 49e5363..b39a960 100644
> --- a/sysdeps/generic/memcopy.h
> +++ b/sysdeps/generic/memcopy.h
> @@ -148,4 +148,6 @@ extern void _wordcopy_bwd_dest_aligned (long int, long int, size_t) __THROW;
>  /* Threshold value for when to enter the unrolled loops.  */
>  #define	OP_T_THRES	16
>  
> +#define MEMCPY_OK_FOR_FWD_MEMMOVE 0
> +
>  #endif /* memcopy.h */
> diff --git a/sysdeps/tile/memcopy.h b/sysdeps/tile/memcopy.h
> index e8326ee..b8ea16a 100644
> --- a/sysdeps/tile/memcopy.h
> +++ b/sysdeps/tile/memcopy.h
> @@ -19,6 +19,8 @@
>  #include <sysdeps/generic/memcopy.h>
>  #include <bits/wordsize.h>
>  
> +#undef MEMCPY_OK_FOR_FWD_MEMMOVE
> +#define MEMCPY_OK_FOR_FWD_MEMMOVE 1
>  /* Support more efficient copying on tilegx32, which supports
>     long long as a native 64-bit type.  */
>  #if defined (__tilegx__) && __WORDSIZE == 32
> diff --git a/sysdeps/tile/tilegx/memmove.c b/sysdeps/tile/tilegx/memmove.c
> deleted file mode 100644
> index 38323ce..0000000
> --- a/sysdeps/tile/tilegx/memmove.c
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/* Copy memory to memory until the specified number of bytes
> -   has been copied.  Overlap is handled correctly.
> -   Copyright (C) 2012-2014 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
> -   <http://www.gnu.org/licenses/>.  */
> -
> -/* The tilegx implementation of memcpy is safe to use for memmove.  */
> -#define MEMCPY_OK_FOR_FWD_MEMMOVE 1
> -#include <string/memmove.c>
Roland McGrath June 27, 2014, 8 p.m. UTC | #5
> On Thu, Jun 26, 2014 at 12:04:23PM -0700, Roland McGrath wrote:
> > That only helps x86_64 and not anybody else using that file (whether
> > directly by sysdeps selection or by #include).  I think memcopy.h should
> > be required to define that one way or the other.  Then we can have a
> > default in sysdeps/generic/memcopy.h.
> 
> Wouldn't that not catch cases where one forgets to define the macro
> and inherits the default?

The norm is not to define it and use the default.  It's not an error to be
caught.  Anyone who does not want the default (today, only tile) would
include the generic header before overriding a subset of its definitions
(as tile does).  So tile would just do:

#undef	MEMCPY_OK_FOR_FWD_MEMMOVE
#define MEMCPY_OK_FOR_FWD_MEMMOVE 1
diff mbox

Patch

diff --git a/sysdeps/x86_64/memmove.c b/sysdeps/x86_64/memmove.c
index 202f5b8..9013c105 100644
--- a/sysdeps/x86_64/memmove.c
+++ b/sysdeps/x86_64/memmove.c
@@ -15,6 +15,7 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#define MEMCPY_OK_FOR_FWD_MEMMOVE 0
 #include "string/memmove.c"
 
 #if !defined memmove && !defined NOT_IN_libc
diff --git a/sysdeps/x86_64/multiarch/memmove.c b/sysdeps/x86_64/multiarch/memmove.c
index ba86e7b..5477be1 100644
--- a/sysdeps/x86_64/multiarch/memmove.c
+++ b/sysdeps/x86_64/multiarch/memmove.c
@@ -37,6 +37,7 @@  extern __typeof (__redirect_memmove) __memmove_ssse3 attribute_hidden;
 extern __typeof (__redirect_memmove) __memmove_ssse3_back attribute_hidden;
 #endif
 
+#define MEMCPY_OK_FOR_FWD_MEMMOVE 0
 #include "string/memmove.c"
 
 #ifndef NOT_IN_libc