Message ID | 20210726083433.385572-1-naohirot@fujitsu.com |
---|---|
State | New |
Headers | show |
Series | config: Remove HAVE_BUILTIN_MEMSET macro | expand |
I'll fix the typo > s patch removed HAVE_BUILTIN_MEMSET macro because GCC 6.2 that is s/s/This/ > minimum requirement to compile glibc already support >__builtin_memset()[1]. Thanks. Naohiro
On Jul 26 2021, Naohiro Tamura via Libc-alpha wrote: > Interestingly, removed code had a critical bug that is > HAVE_BUILTIN_MEMSET macro never be defined, because yes/no assignment > to libc_cv_gcc_builtin_memset was reversed in configure.ac as below: No, the point of the check is that __buildin_memset does *not* expand to a memset libcall, as explained in the comment you removed. Andreas.
Hi Andreas, Thank you for the comment. > From: Andreas Schwab <schwab@linux-m68k.org> > On Jul 26 2021, Naohiro Tamura via Libc-alpha wrote: > > > Interestingly, removed code had a critical bug that is > > HAVE_BUILTIN_MEMSET macro never be defined, because yes/no assignment > > to libc_cv_gcc_builtin_memset was reversed in configure.ac as below: > > No, the point of the check is that __buildin_memset does *not* expand to > a memset libcall, as explained in the comment you removed. > Is the comment you mentioned below? /* 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. */ + make function calls at this point. Use '__builtin_memset' instead. + 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. */ Do you mean that yes/no assignment to libc_cv_gcc_builtin_memset was NOT reversed? If it was not reversed, config.h never had " #define HAVE_BUILTIN_MEMSET 1" even if gcc 8.3 is used. Thanks. Naohiro
On Jul 26 2021, naohirot@fujitsu.com wrote: > If it was not reversed, config.h never had " #define HAVE_BUILTIN_MEMSET 1" > even if gcc 8.3 is used. This is correct if the builtin just expands to a memset libcall. Andreas.
Hi Andreas, Thanks for the explanation. > From: Andreas Schwab <schwab@linux-m68k.org> > > If it was not reversed, config.h never had " #define HAVE_BUILTIN_MEMSET 1" > > even if gcc 8.3 is used. > > This is correct if the builtin just expands to a memset libcall. Now I understood that by looking at the grep argument again in the line below: 1519 if AC_TRY_COMMAND([${CC-cc} -O3 -S conftest.c -o - | grep -F "memset" > /dev/null]); So the macro name "HAVE_BUILTIN_MEMSET" is very confusing, since GCC 6.2 manual mentions that __builtin_memset is supported. Can we agree to change it to "HAVE_NON_LIB_EXPAND_BUILTIN_MEMSET" or "HAVE_NON_LIBCALL_BUILTIN_MEMSET"? Thanks. Naohiro
diff --git a/config.h.in b/config.h.in index 8b45a3a61d77..4647632f2632 100644 --- a/config.h.in +++ b/config.h.in @@ -40,9 +40,6 @@ 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 compiler accepts -ftree-loop-distribute-patterns. */ #undef HAVE_CC_INHIBIT_LOOP_TO_LIBCALL diff --git a/configure b/configure index 9619c10991d0..6f85d28ea085 100755 --- a/configure +++ b/configure @@ -6261,37 +6261,6 @@ if test $libc_cv_have_section_quotes = yes; then 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 : - $as_echo_n "(cached) " >&6 -else - cat > conftest.c <<\EOF -void zero (void *x) -{ - __builtin_memset (x, 0, 1000); -} -EOF -if { ac_try='${CC-cc} -O3 -S conftest.c -o - | grep -F "memset" > /dev/null' - { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 - (eval $ac_try) 2>&5 - ac_status=$? - $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 - test $ac_status = 0; }; }; -then - libc_cv_gcc_builtin_memset=no -else - libc_cv_gcc_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 - -fi - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for redirection of built-in functions" >&5 $as_echo_n "checking for redirection of built-in functions... " >&6; } if ${libc_cv_gcc_builtin_redirection+:} false; then : diff --git a/configure.ac b/configure.ac index 34ecbba54054..0c5ee6623c4c 100644 --- a/configure.ac +++ b/configure.ac @@ -1508,25 +1508,6 @@ 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 -cat > conftest.c <<\EOF -void zero (void *x) -{ - __builtin_memset (x, 0, 1000); -} -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 -else - libc_cv_gcc_builtin_memset=yes -fi -rm -f conftest* ]) -if test "$libc_cv_gcc_builtin_memset" = yes ; then - AC_DEFINE(HAVE_BUILTIN_MEMSET) -fi - AC_CACHE_CHECK(for redirection of built-in functions, libc_cv_gcc_builtin_redirection, [dnl cat > conftest.c <<\EOF extern char *strstr (const char *, const char *) __asm ("my_strstr"); diff --git a/elf/rtld.c b/elf/rtld.c index d733359eaf80..d0da99bd6d78 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -526,19 +526,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. */ + make function calls at this point. Use '__builtin_memset' instead. + 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 __builtin_memset (bootstrap_map.l_info, '\0', sizeof (bootstrap_map.l_info)); -# else - for (size_t cnt = 0; - cnt < sizeof (bootstrap_map.l_info) / sizeof (bootstrap_map.l_info[0]); - ++cnt) - bootstrap_map.l_info[cnt] = 0; -# endif #endif /* Figure out the run-time load address of the dynamic linker itself. */