This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: PING: PATCH: Automatically test IFUNC implementations
On Sat, Oct 6, 2012 at 9:59 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Oct 05, 2012 at 03:47:49PM -0700, Roland McGrath wrote:
>> As always, it's really easiest if the final version of the patches are
>> posted in the normal fashion for review.
>>
>> Looking at the branch, some things that need fixing. After fixing these,
>> please post the patch series again in the proper way for final review.
>>
>> * TYpo in log entry.
>
> Fixed.
>
>> * array-size arg to __libc_func should be size_t, return value also size_t
>
> Fixed.
>
>> * __libc_func is a poor name, make it something more verbose,
>> e.g. __libc_enumerate_implementations_for_test
>
> I changed it to __libc_supported_implementations.
>
>> * libc-func doesn't belong in string/Makefile and string/Versions.
>> It's a generic thing. Put it someplace like misc/ instead.
>
> Fixed.
>
>> * C files don't belong in sysdeps/generic/ without special reason.
>> Put it in the same directory whose Makefile lists it, like all stubs are.
>
> Fixed.
>
>> * There is no real need to use %ifdef in Versions.
>> Naming a nonexistent symbol doesn't hurt. Just drop the conditional.
>
> Fixed.
>
>> * 'sizeof foo', not 'sizeof (foo)' when foo is not a type name.
>
> Fixed.
>
>> * All new files need a top line that's a descriptive comment.
>
> Fixed.
>
>> * __attribute_used__ is wrong for unreferenced parameters.
>> Use __attribute__ ((unused)) instead.
>
> Fixed.
>
> Here is the updated patch to add framework to test IFUNC implementations
> on target. OK to install?
>
>
>> Then each one looks like:
>> FUNC_FINDER (__memmove_chk,
>> FIND_FUNC (HAS_SSSE3, __memmove_chk_ssse3_back),
>> FIND_FUNC (HAS_SSSE3, __memmove_chk_ssse3),
>> FIND_FUNC (1, __memmove_chk_sse2)
>> )
>>
>
> That is a good idea. I will implement something along this line for
> i686 and x86-64.
>
> Thanks.
>
>
> H.J.
> ---
> From e6c6fa2b14059e6dc4d5fb4007598c8e0cfe8335 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Sat, 6 Oct 2012 09:54:25 -0700
> Subject: [PATCH 1/2] Framework to test IFUNC implementations on target
>
> ---
> ChangeLog | 22 +++++++++++++++++++++
> Rules | 5 +++++
> include/libc-func.h | 43 ++++++++++++++++++++++++++++++++++++++++
> misc/Makefile | 4 ++++
> misc/Versions | 1 +
> misc/libc-func.c | 32 ++++++++++++++++++++++++++++++
> string/test-string.h | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 7 files changed, 162 insertions(+), 1 deletion(-)
> create mode 100644 include/libc-func.h
> create mode 100644 misc/libc-func.c
>
> diff --git a/ChangeLog b/ChangeLog
> index 1b2a455..af97647 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,25 @@
> +2012-10-08 H.J. Lu <hongjiu.lu@intel.com>
> +
> + * Rules (tests): Filter out $(tests-ifunc) if multi-arch isn't
> + enabled.
> + (xtests): Filter out $(xtests-ifunc) if multi-arch isn't enabled.
> + * include/libc-func.h: New file.
> + * misc/libc-func.c: Likewise.
> + * misc/Makefile (routines): Add libc-func if multi-arch is
> + enabled.
> + * misc/Versions: Add __libc_supported_implementations to
> + GLIBC_PRIVATE
> + * string/test-string.h: Include <libc-func.h>.
> + (func_list): New. Defined only if TEST_IFUNC and TEST_NAME are
> + defined.
> + (func_count): Likewise.
> + (impl_count): Likewise.
> + (impl_array): Likewise.
> + (FOR_EACH_IMPL): Support func_list if TEST_IFUNC and TEST_NAME
> + are defined.
> + (test_init): Call __libc_supported_implementations to initialize
> + func_list if TEST_IFUNC and TEST_NAME are defined.
> +
> 2012-10-05 David S. Miller <davem@davemloft.net>
>
> * sysdeps/sparc/sparc64/multiarch/memset-niagara4.S: New file.
> diff --git a/Rules b/Rules
> index 17d938e..5e33610 100644
> --- a/Rules
> +++ b/Rules
> @@ -84,6 +84,11 @@ common-generated += dummy.o dummy.c
> # This makes all the auxiliary and test programs.
>
> .PHONY: others tests
> +ifeq ($(multi-arch),no)
> +tests := $(filter-out $(tests-ifunc), $(tests))
> +xtests := $(filter-out $(xtests-ifunc), $(xtests))
> +endif
> +
> ifeq ($(build-programs),yes)
> others: $(addprefix $(objpfx),$(others) $(sysdep-others) $(extra-objs))
> else
> diff --git a/include/libc-func.h b/include/libc-func.h
> new file mode 100644
> index 0000000..72a358e
> --- /dev/null
> +++ b/include/libc-func.h
> @@ -0,0 +1,43 @@
> +/* Internal header file for __libc_supported_implementations.
> + Copyright (C) 2012 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/>. */
> +
> +#ifndef _LIBC_FUNC_H
> +#define _LIBC_FUNC_H 1
> +
> +#include <stddef.h>
> +
> +struct libc_func_test
> +{
> + /* The name of function to be tested. */
> + const char *name;
> + /* The address of function to be tested. */
> + void (*fn) (void);
> +};
> +
> +/* Dynamic initializer for struct libc_func_test entry with FN. */
> +#define LIBC_FUNC_INIT(fn) \
> + (struct libc_func_test) { #fn, (void (*) (void)) fn }
> +
> +/* Fill ARRAY of MAX elements with IFUNC implementations for function
> + NAME supported on target machine and return the number of valid
> + entries. */
> +extern size_t __libc_supported_implementations (const char *name,
> + struct libc_func_test *array,
> + size_t max);
> +
> +#endif /* libc-func.h */
> diff --git a/misc/Makefile b/misc/Makefile
> index e5e9b9f..f51f407 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -71,6 +71,10 @@ generated := tst-error1.mtrace tst-error1-mem
>
> include ../Makeconfig
>
> +ifneq ($(multi-arch),no)
> +routines += libc-func
> +endif
> +
> aux := init-misc
> install-lib := libbsd-compat.a libg.a
> gpl2lgpl := error.c error.h
> diff --git a/misc/Versions b/misc/Versions
> index b2a9147..8c898e0 100644
> --- a/misc/Versions
> +++ b/misc/Versions
> @@ -151,5 +151,6 @@ libc {
> }
> GLIBC_PRIVATE {
> __madvise;
> + __libc_supported_implementations;
> }
> }
> diff --git a/misc/libc-func.c b/misc/libc-func.c
> new file mode 100644
> index 0000000..2086e69
> --- /dev/null
> +++ b/misc/libc-func.c
> @@ -0,0 +1,32 @@
> +/* Default __libc_supported_implementations.
> + Copyright (C) 2012 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/>. */
> +
> +#include <libc-func.h>
> +
> +/* Fill ARRAY of MAX elements with IFUNC implementations for function
> + NAME supported on target machine and return the number of valid
> + entries. */
> +
> +size_t
> +__libc_supported_implementations
> + (const char *name __attribute__ ((unused)),
> + struct libc_func_test *array __attribute__ ((unused)),
> + size_t max __attribute__ ((unused)))
> +{
> + return 0;
> +}
> diff --git a/string/test-string.h b/string/test-string.h
> index c94d822..2492152 100644
> --- a/string/test-string.h
> +++ b/string/test-string.h
> @@ -50,6 +50,7 @@ extern impl_t __start_impls[], __stop_impls[];
> #include <error.h>
> #include <errno.h>
> #include <time.h>
> +#include <libc-func.h>
> #define GL(x) _##x
> #define GLRO(x) _##x
> #include <hp-timing.h>
> @@ -106,9 +107,56 @@ size_t iterations = 100000;
> #define CALL(impl, ...) \
> (* (proto_t) (impl)->fn) (__VA_ARGS__)
>
> -#define FOR_EACH_IMPL(impl, notall) \
> +#if defined TEST_IFUNC && defined TEST_NAME
> +/* Increase size of FUNC_LIST if assert is triggered at run-time. */
> +static struct libc_func_test func_list[32];
> +static int func_count;
> +static int impl_count = -1;
> +static impl_t *impl_array;
> +
> +# define FOR_EACH_IMPL(impl, notall) \
> + impl_t *impl; \
> + int count; \
> + if (impl_count == -1) \
> + { \
> + impl_count = 0; \
> + if (func_count != 0) \
> + { \
> + int f; \
> + impl_t *skip = NULL, *a; \
> + for (impl = __start_impls; impl < __stop_impls; ++impl) \
> + if (strcmp (impl->name, TEST_NAME) == 0) \
> + skip = impl; \
> + else \
> + impl_count++; \
> + impl_count += func_count; \
> + a = impl_array = malloc (impl_count * sizeof (impl_t)); \
> + for (impl = __start_impls; impl < __stop_impls; ++impl) \
> + if (impl != skip) \
> + *a++ = *impl; \
> + for (f = 0; f < func_count; f++) \
> + { \
> + a->name = func_list[f].name; \
> + a->fn = func_list[f].fn; \
> + a->test = 1; \
> + a++; \
> + } \
> + } \
> + else \
> + { \
> + for (impl = __start_impls; impl < __stop_impls; ++impl) \
> + impl_count++; \
> + impl_array = __start_impls; \
> + } \
> + } \
> + impl = impl_array; \
> + for (count = 0; count < impl_count; ++count, ++impl) \
> + if (!notall || impl->test)
> +#else
> +# define FOR_EACH_IMPL(impl, notall) \
> for (impl_t *impl = __start_impls; impl < __stop_impls; ++impl) \
> if (!notall || impl->test)
> +#endif
>
> #define HP_TIMING_BEST(best_time, start, end) \
> do \
> @@ -127,6 +175,12 @@ size_t iterations = 100000;
> static void
> test_init (void)
> {
> +#if defined TEST_IFUNC && defined TEST_NAME
> +# define SIZEOF_ARRAY(array) (sizeof array / sizeof array[0])
> + func_count = __libc_supported_implementations (TEST_NAME, func_list,
> + SIZEOF_ARRAY (func_list));
> +#endif
> +
> page_size = 2 * getpagesize ();
> #ifdef MIN_PAGE_SIZE
> if (page_size < MIN_PAGE_SIZE)
> --
> 1.7.11.4
>
Is this OK?
--
H.J.