Message ID | 20210805075207.433697-1-naohirot@fujitsu.com |
---|---|
State | New |
Headers | show |
Series | benchtests: Add memset zero fill benchmark test | expand |
On 05/08/2021 04:52, Naohiro Tamura via Libc-alpha wrote: > This patch renames HAVE_BUILTIN_MEMSET macro to > HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET. > > The name "HAVE_BUILTIN_MEMSET" is very confusing. > This macro cannot be removed even though GCC 6.2, that is minimum > requirement to compile glibc, already supports __builtin_memset[1]. > It doesn't indicate whether GCC supports __builtin_memset or not. > > But it indicates whether GCC supports __builtin_memset which doesn't > expand to a memset libcall or not. > > Therefor HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET is more appropriate to > increase code readability. > > [1] https://gcc.gnu.org/onlinedocs/gcc-6.2.0/gcc/Other-Builtins.html Sorry, but I don't see much gain in renaming the internal variable. I agree with the comment improvement. > --- > config.h.in | 4 ++-- > configure | 14 +++++++------- > configure.ac | 10 +++++----- > elf/rtld.c | 9 +++++---- > 4 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/config.h.in b/config.h.in > index 8b45a3a61d77..4700dc9eba9b 100644 > --- a/config.h.in > +++ b/config.h.in > @@ -40,8 +40,8 @@ > shared between GNU libc and GNU gettext projects. */ > #define HAVE_BUILTIN_EXPECT 1 > > -/* Define if the compiler supports __builtin_memset. */ > -#undef HAVE_BUILTIN_MEMSET > +/* Define if the compiler supports non lib expand __builtin_memset. */ > +#undef HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET > > /* Define if compiler accepts -ftree-loop-distribute-patterns. */ > #undef HAVE_CC_INHIBIT_LOOP_TO_LIBCALL Ok. > diff --git a/configure b/configure > index 9619c10991d0..224c754cf466 100755 > --- a/configure > +++ b/configure > @@ -6263,7 +6263,7 @@ fi > > { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_memset" >&5 > $as_echo_n "checking for __builtin_memset... " >&6; } > -if ${libc_cv_gcc_builtin_memset+:} false; then : > +if ${libc_cv_gcc_non_lib_expand_builtin_memset+:} false; then : > $as_echo_n "(cached) " >&6 > else > cat > conftest.c <<\EOF > @@ -6279,16 +6279,16 @@ if { ac_try='${CC-cc} -O3 -S conftest.c -o - | grep -F "memset" > /dev/null' > $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 > test $ac_status = 0; }; }; > then > - libc_cv_gcc_builtin_memset=no > + libc_cv_gcc_non_lib_expand_builtin_memset=no > else > - libc_cv_gcc_builtin_memset=yes > + libc_cv_gcc_non_lib_expand_builtin_memset=yes > fi > rm -f conftest* > fi > -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_builtin_memset" >&5 > -$as_echo "$libc_cv_gcc_builtin_memset" >&6; } > -if test "$libc_cv_gcc_builtin_memset" = yes ; then > - $as_echo "#define HAVE_BUILTIN_MEMSET 1" >>confdefs.h > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_non_lib_expand_builtin_memset" >&5 > +$as_echo "$libc_cv_gcc_non_lib_expand_builtin_memset" >&6; } > +if test "$libc_cv_gcc_non_lib_expand_builtin_memset" = yes ; then > + $as_echo "#define HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET 1" >>confdefs.h > > fi > > diff --git a/configure.ac b/configure.ac > index 34ecbba54054..451cea7683e6 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1508,7 +1508,7 @@ if test $libc_cv_have_section_quotes = yes; then > AC_DEFINE(HAVE_SECTION_QUOTES) > fi > > -AC_CACHE_CHECK(for __builtin_memset, libc_cv_gcc_builtin_memset, [dnl > +AC_CACHE_CHECK(for __builtin_memset, libc_cv_gcc_non_lib_expand_builtin_memset, [dnl > cat > conftest.c <<\EOF > void zero (void *x) > { > @@ -1518,13 +1518,13 @@ EOF > dnl > if AC_TRY_COMMAND([${CC-cc} -O3 -S conftest.c -o - | grep -F "memset" > /dev/null]); > then > - libc_cv_gcc_builtin_memset=no > + libc_cv_gcc_non_lib_expand_builtin_memset=no > else > - libc_cv_gcc_builtin_memset=yes > + libc_cv_gcc_non_lib_expand_builtin_memset=yes > fi > rm -f conftest* ]) > -if test "$libc_cv_gcc_builtin_memset" = yes ; then > - AC_DEFINE(HAVE_BUILTIN_MEMSET) > +if test "$libc_cv_gcc_non_lib_expand_builtin_memset" = yes ; then > + AC_DEFINE(HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET) > fi > > AC_CACHE_CHECK(for redirection of built-in functions, libc_cv_gcc_builtin_redirection, [dnl I see no point in rename it, a better comment as you are doing is suffice. > diff --git a/elf/rtld.c b/elf/rtld.c > index d733359eaf80..a18494fcd38e 100644 > --- a/elf/rtld.c > +++ b/elf/rtld.c > @@ -527,11 +527,12 @@ _dl_start (void *arg) > /* Partly clean the `bootstrap_map' structure up. Don't use > `memset' since it might not be built in or inlined and we cannot > make function calls at this point. Use '__builtin_memset' if we > - know it is available. We do not have to clear the memory if we > - do not have to use the temporary bootstrap_map. Global variables > - are initialized to zero by default. */ > + know it is available and does not expand to a memset libcall. > + We do not have to clear the memory if we do not have to use the > + temporary bootstrap_map. Global variables are initialized to > + zero by default. */ > #ifndef DONT_USE_BOOTSTRAP_MAP > -# ifdef HAVE_BUILTIN_MEMSET > +# ifdef HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET > __builtin_memset (bootstrap_map.l_info, '\0', sizeof (bootstrap_map.l_info)); > # else > for (size_t cnt = 0; > Ok.
diff --git a/config.h.in b/config.h.in index 8b45a3a61d77..4700dc9eba9b 100644 --- a/config.h.in +++ b/config.h.in @@ -40,8 +40,8 @@ shared between GNU libc and GNU gettext projects. */ #define HAVE_BUILTIN_EXPECT 1 -/* Define if the compiler supports __builtin_memset. */ -#undef HAVE_BUILTIN_MEMSET +/* Define if the compiler supports non lib expand __builtin_memset. */ +#undef HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET /* Define if compiler accepts -ftree-loop-distribute-patterns. */ #undef HAVE_CC_INHIBIT_LOOP_TO_LIBCALL diff --git a/configure b/configure index 9619c10991d0..224c754cf466 100755 --- a/configure +++ b/configure @@ -6263,7 +6263,7 @@ fi { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_memset" >&5 $as_echo_n "checking for __builtin_memset... " >&6; } -if ${libc_cv_gcc_builtin_memset+:} false; then : +if ${libc_cv_gcc_non_lib_expand_builtin_memset+:} false; then : $as_echo_n "(cached) " >&6 else cat > conftest.c <<\EOF @@ -6279,16 +6279,16 @@ if { ac_try='${CC-cc} -O3 -S conftest.c -o - | grep -F "memset" > /dev/null' $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 test $ac_status = 0; }; }; then - libc_cv_gcc_builtin_memset=no + libc_cv_gcc_non_lib_expand_builtin_memset=no else - libc_cv_gcc_builtin_memset=yes + libc_cv_gcc_non_lib_expand_builtin_memset=yes fi rm -f conftest* fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_builtin_memset" >&5 -$as_echo "$libc_cv_gcc_builtin_memset" >&6; } -if test "$libc_cv_gcc_builtin_memset" = yes ; then - $as_echo "#define HAVE_BUILTIN_MEMSET 1" >>confdefs.h +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_non_lib_expand_builtin_memset" >&5 +$as_echo "$libc_cv_gcc_non_lib_expand_builtin_memset" >&6; } +if test "$libc_cv_gcc_non_lib_expand_builtin_memset" = yes ; then + $as_echo "#define HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET 1" >>confdefs.h fi diff --git a/configure.ac b/configure.ac index 34ecbba54054..451cea7683e6 100644 --- a/configure.ac +++ b/configure.ac @@ -1508,7 +1508,7 @@ if test $libc_cv_have_section_quotes = yes; then AC_DEFINE(HAVE_SECTION_QUOTES) fi -AC_CACHE_CHECK(for __builtin_memset, libc_cv_gcc_builtin_memset, [dnl +AC_CACHE_CHECK(for __builtin_memset, libc_cv_gcc_non_lib_expand_builtin_memset, [dnl cat > conftest.c <<\EOF void zero (void *x) { @@ -1518,13 +1518,13 @@ EOF dnl if AC_TRY_COMMAND([${CC-cc} -O3 -S conftest.c -o - | grep -F "memset" > /dev/null]); then - libc_cv_gcc_builtin_memset=no + libc_cv_gcc_non_lib_expand_builtin_memset=no else - libc_cv_gcc_builtin_memset=yes + libc_cv_gcc_non_lib_expand_builtin_memset=yes fi rm -f conftest* ]) -if test "$libc_cv_gcc_builtin_memset" = yes ; then - AC_DEFINE(HAVE_BUILTIN_MEMSET) +if test "$libc_cv_gcc_non_lib_expand_builtin_memset" = yes ; then + AC_DEFINE(HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET) fi AC_CACHE_CHECK(for redirection of built-in functions, libc_cv_gcc_builtin_redirection, [dnl diff --git a/elf/rtld.c b/elf/rtld.c index d733359eaf80..a18494fcd38e 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -527,11 +527,12 @@ _dl_start (void *arg) /* Partly clean the `bootstrap_map' structure up. Don't use `memset' since it might not be built in or inlined and we cannot make function calls at this point. Use '__builtin_memset' if we - know it is available. We do not have to clear the memory if we - do not have to use the temporary bootstrap_map. Global variables - are initialized to zero by default. */ + know it is available and does not expand to a memset libcall. + We do not have to clear the memory if we do not have to use the + temporary bootstrap_map. Global variables are initialized to + zero by default. */ #ifndef DONT_USE_BOOTSTRAP_MAP -# ifdef HAVE_BUILTIN_MEMSET +# ifdef HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET __builtin_memset (bootstrap_map.l_info, '\0', sizeof (bootstrap_map.l_info)); # else for (size_t cnt = 0;