This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Move bench target into benchtests
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: Roland McGrath <roland at hack dot frob dot com>, libc-alpha at sourceware dot org
- Date: Tue, 09 Apr 2013 09:38:27 -0400
- Subject: Re: [PATCH] Move bench target into benchtests
- References: <20130403080526 dot GA20842 at spoyarek dot pnq dot redhat dot com> <20130408220956 dot 124492C09F at topped-with-meat dot com> <20130409044525 dot GD15689 at spoyarek dot pnq dot redhat dot com>
On 04/09/2013 12:45 AM, Siddhesh Poyarekar wrote:
> On Mon, Apr 08, 2013 at 03:09:56PM -0700, Roland McGrath wrote:
>> Why is any of this stuff in Rules at all? It appears to be relevant only
>> to bench/Makefile and so it should be there. Is there some intent to
>> support the 'bench' variable in other subdir makefiles?
>
> Right, there is no reason to have it in the root directory. Here's a
> patch to move the rules within benchtests. Looks OK?
>
> Siddhesh
>
> * Rules (bench): Move target definition...
> * benchtests/Rules: ... into this new file.
> * benchtests/Makefile: Adjust.
Looks good to me. One nit below.
> diff --git a/Rules b/Rules
> index d4a0027..86a0520 100644
> --- a/Rules
> +++ b/Rules
> @@ -189,36 +189,6 @@ $(objpfx)%.out: /dev/null $(objpfx)% # Make it 2nd arg for canned sequence.
>
> endif # tests
>
> -# Build and run benchmark programs.
> -binaries-bench := $(addprefix $(objpfx)bench-,$(bench))
> -
> -run-bench = $(test-wrapper-env) \
> - GCONV_PATH=$(common-objpfx)iconvdata LC_ALL=C \
> - $($*-ENV) $(run-via-rtld-prefix) $${run}
> -
> -bench: $(binaries-bench)
> - for run in $^; do \
> - echo "Running $${run}"; \
> - eval $(run-bench) >> $(objpfx)bench.out-tmp; \
> - done; \
> - if [ -f $(objpfx)bench.out ]; then \
> - mv -f $(objpfx)bench.out $(objpfx)bench.out.old; \
> - fi; \
> - mv -f $(objpfx)bench.out-tmp $(objpfx)bench.out
> -
> -$(binaries-bench): %: %.o \
> - $(sort $(filter $(common-objpfx)lib%,$(link-libc))) \
> - $(addprefix $(csu-objpfx),start.o) $(+preinit) $(+postinit)
> - $(+link)
> -
> -$(objpfx)bench-%.c: %-inputs bench-skeleton.c
> - { if [ -n "$($*-INCLUDE)" ]; then \
> - cat $($*-INCLUDE); \
> - fi; \
> - $(..)scripts/bench.pl $(patsubst %-inputs,%,$<) \
> - $($*-ITER) $($*-ARGLIST) $($*-RET); } > $@-tmp
> - mv -f $@-tmp $@
> -
>
> .PHONY: distclean realclean subdir_distclean subdir_realclean \
> subdir_clean subdir_mostlyclean subdir_testclean
> diff --git a/benchtests/Makefile b/benchtests/Makefile
> index a6a9299..d3d5253 100644
> --- a/benchtests/Makefile
> +++ b/benchtests/Makefile
> @@ -105,4 +105,4 @@ slowatan-INCLUDE = slowatan.c
> LDFLAGS-bench-slowatan = -lm
>
> include ../Makeconfig
> -include ../Rules
> +include Rules
> diff --git a/benchtests/Rules b/benchtests/Rules
> new file mode 100644
> index 0000000..4715084
> --- /dev/null
> +++ b/benchtests/Rules
> @@ -0,0 +1,53 @@
> +# Copyright (C) 2013 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/>.
> +
> +# Rules to build and run benchmark programs.
> +
> +include ../Rules
> +
> +binaries-bench := $(addprefix $(objpfx)bench-,$(bench))
> +
> +run-bench = $(test-wrapper-env) \
> + GCONV_PATH=$(common-objpfx)iconvdata LC_ALL=C \
> + $($*-ENV) $(run-via-rtld-prefix) $${run}
> +
> +bench: $(binaries-bench)
> + for run in $^; do \
> + echo "Running $${run}"; \
> + eval $(run-bench) >> $(objpfx)bench.out-tmp; \
Do you need `eval' here? Schwab caught this in my implementation also,
and I didn't need it because all the variables were resolved by the time
the command is evaluated.
> + done; \
> + if [ -f $(objpfx)bench.out ]; then \
> + mv -f $(objpfx)bench.out $(objpfx)bench.out.old; \
> + fi; \
> + mv -f $(objpfx)bench.out-tmp $(objpfx)bench.out
> +
> +$(binaries-bench): %: %.o \
> + $(sort $(filter $(common-objpfx)lib%,$(link-libc))) \
> + $(addprefix $(csu-objpfx),start.o) $(+preinit) $(+postinit)
> + $(+link)
> +
> +$(objpfx)bench-%.c: %-inputs bench-skeleton.c
> + { if [ -n "$($*-INCLUDE)" ]; then \
> + cat $($*-INCLUDE); \
> + fi; \
> + $(..)scripts/bench.pl $(patsubst %-inputs,%,$<) \
> + $($*-ITER) $($*-ARGLIST) $($*-RET); } > $@-tmp
> + mv -f $@-tmp $@
> +
> +# Local Variables:
> +# mode: makefile
> +# End:
>
Cheers,
Carlos.