* [PATCH 1/3] cdefs.h: Define __COLD @ 2023-04-29 13:12 Sergey Bugaev 2023-04-29 13:12 ` [PATCH 2/3] hurd: Mark error functions as __COLD Sergey Bugaev ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Sergey Bugaev @ 2023-04-29 13:12 UTC (permalink / raw) To: libc-alpha; +Cc: bug-hurd, Samuel Thibault This expands to __attribute__ ((cold)) when supported. It should be used to mark up functions that are invoked rarely. Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- I can change __COLD to __attribute_cold__ if that is preferred. misc/sys/cdefs.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index 23ec0ebd..8eec4b94 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -98,6 +98,12 @@ # endif # endif +# if __GNUC_PREREQ (4, 3) || __glibc_has_attribute (__cold__) +# define __COLD __attribute__ ((__cold__)) +# else +# define __COLD +# endif + #else /* Not GCC or clang. */ # if (defined __cplusplus \ -- 2.40.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] hurd: Mark error functions as __COLD 2023-04-29 13:12 [PATCH 1/3] cdefs.h: Define __COLD Sergey Bugaev @ 2023-04-29 13:12 ` Sergey Bugaev 2023-04-29 15:04 ` Samuel Thibault 2023-04-29 13:12 ` [PATCH 3/3] Mark various cold " Sergey Bugaev 2023-04-29 13:20 ` [PATCH] cdefs.h: Define __COLD Sergey Bugaev 2 siblings, 1 reply; 12+ messages in thread From: Sergey Bugaev @ 2023-04-29 13:12 UTC (permalink / raw) To: libc-alpha; +Cc: bug-hurd, Samuel Thibault This should hopefully hint the compiler that they are unlikely to be called. Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- hurd/hurd.h | 2 +- hurd/hurd/fd.h | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hurd/hurd.h b/hurd/hurd.h index eca4abb7..06f00e93 100644 --- a/hurd/hurd.h +++ b/hurd/hurd.h @@ -48,7 +48,7 @@ #define _HURD_H_EXTERN_INLINE __extern_inline #endif -extern int __hurd_fail (error_t err); +extern int __hurd_fail (error_t err) __COLD; #ifdef __USE_EXTERN_INLINES _HURD_H_EXTERN_INLINE int diff --git a/hurd/hurd/fd.h b/hurd/hurd/fd.h index f6139544..241797bf 100644 --- a/hurd/hurd/fd.h +++ b/hurd/hurd/fd.h @@ -149,7 +149,7 @@ _hurd_fd_get (int fd) /* Check if ERR should generate a signal. Returns the signal to take, or zero if none. */ -extern int _hurd_fd_error_signal (error_t err); +extern int _hurd_fd_error_signal (error_t err) __COLD; #ifdef __USE_EXTERN_INLINES _HURD_FD_H_EXTERN_INLINE int @@ -174,7 +174,7 @@ _hurd_fd_error_signal (error_t err) always use this function to handle errors from RPCs made on file descriptor ports. Some errors are translated into signals. */ -extern error_t _hurd_fd_error (int fd, error_t err); +extern error_t _hurd_fd_error (int fd, error_t err) __COLD; #ifdef __USE_EXTERN_INLINES _HURD_FD_H_EXTERN_INLINE error_t @@ -194,7 +194,7 @@ _hurd_fd_error (int fd, error_t err) /* Handle error code ERR from an RPC on file descriptor FD's port. Set `errno' to the appropriate error code, and always return -1. */ -extern int __hurd_dfail (int fd, error_t err); +extern int __hurd_dfail (int fd, error_t err) __COLD; #ifdef __USE_EXTERN_INLINES _HURD_FD_H_EXTERN_INLINE int @@ -208,7 +208,7 @@ __hurd_dfail (int fd, error_t err) /* Likewise, but do not raise SIGPIPE on EPIPE if flags contain MSG_NOSIGNAL. */ -extern int __hurd_sockfail (int fd, int flags, error_t err); +extern int __hurd_sockfail (int fd, int flags, error_t err) __COLD; #ifdef __USE_EXTERN_INLINES _HURD_FD_H_EXTERN_INLINE int -- 2.40.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] hurd: Mark error functions as __COLD 2023-04-29 13:12 ` [PATCH 2/3] hurd: Mark error functions as __COLD Sergey Bugaev @ 2023-04-29 15:04 ` Samuel Thibault 0 siblings, 0 replies; 12+ messages in thread From: Samuel Thibault @ 2023-04-29 15:04 UTC (permalink / raw) To: Sergey Bugaev; +Cc: libc-alpha, bug-hurd Applied, thanks! Sergey Bugaev, le sam. 29 avril 2023 16:12:22 +0300, a ecrit: > This should hopefully hint the compiler that they are unlikely > to be called. > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > hurd/hurd.h | 2 +- > hurd/hurd/fd.h | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hurd/hurd.h b/hurd/hurd.h > index eca4abb7..06f00e93 100644 > --- a/hurd/hurd.h > +++ b/hurd/hurd.h > @@ -48,7 +48,7 @@ > #define _HURD_H_EXTERN_INLINE __extern_inline > #endif > > -extern int __hurd_fail (error_t err); > +extern int __hurd_fail (error_t err) __COLD; > > #ifdef __USE_EXTERN_INLINES > _HURD_H_EXTERN_INLINE int > diff --git a/hurd/hurd/fd.h b/hurd/hurd/fd.h > index f6139544..241797bf 100644 > --- a/hurd/hurd/fd.h > +++ b/hurd/hurd/fd.h > @@ -149,7 +149,7 @@ _hurd_fd_get (int fd) > /* Check if ERR should generate a signal. > Returns the signal to take, or zero if none. */ > > -extern int _hurd_fd_error_signal (error_t err); > +extern int _hurd_fd_error_signal (error_t err) __COLD; > > #ifdef __USE_EXTERN_INLINES > _HURD_FD_H_EXTERN_INLINE int > @@ -174,7 +174,7 @@ _hurd_fd_error_signal (error_t err) > always use this function to handle errors from RPCs made on file > descriptor ports. Some errors are translated into signals. */ > > -extern error_t _hurd_fd_error (int fd, error_t err); > +extern error_t _hurd_fd_error (int fd, error_t err) __COLD; > > #ifdef __USE_EXTERN_INLINES > _HURD_FD_H_EXTERN_INLINE error_t > @@ -194,7 +194,7 @@ _hurd_fd_error (int fd, error_t err) > /* Handle error code ERR from an RPC on file descriptor FD's port. > Set `errno' to the appropriate error code, and always return -1. */ > > -extern int __hurd_dfail (int fd, error_t err); > +extern int __hurd_dfail (int fd, error_t err) __COLD; > > #ifdef __USE_EXTERN_INLINES > _HURD_FD_H_EXTERN_INLINE int > @@ -208,7 +208,7 @@ __hurd_dfail (int fd, error_t err) > /* Likewise, but do not raise SIGPIPE on EPIPE if flags contain > MSG_NOSIGNAL. */ > > -extern int __hurd_sockfail (int fd, int flags, error_t err); > +extern int __hurd_sockfail (int fd, int flags, error_t err) __COLD; > > #ifdef __USE_EXTERN_INLINES > _HURD_FD_H_EXTERN_INLINE int > -- > 2.40.1 > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] Mark various cold functions as __COLD 2023-04-29 13:12 [PATCH 1/3] cdefs.h: Define __COLD Sergey Bugaev 2023-04-29 13:12 ` [PATCH 2/3] hurd: Mark error functions as __COLD Sergey Bugaev @ 2023-04-29 13:12 ` Sergey Bugaev 2023-04-29 15:06 ` Samuel Thibault 2023-05-01 21:52 ` Cristian Rodríguez 2023-04-29 13:20 ` [PATCH] cdefs.h: Define __COLD Sergey Bugaev 2 siblings, 2 replies; 12+ messages in thread From: Sergey Bugaev @ 2023-04-29 13:12 UTC (permalink / raw) To: libc-alpha; +Cc: bug-hurd, Samuel Thibault GCC docs explicitly list perror () as a good candidate for using __attribute__ ((cold)). So apply __COLD to perror () and similar functions. Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- include/error.h | 4 ++-- libio/stdio.h | 2 +- misc/err.h | 12 ++++++------ misc/error.h | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/error.h b/include/error.h index 9e96262f..4db67cba 100644 --- a/include/error.h +++ b/include/error.h @@ -5,11 +5,11 @@ void __error_internal (int status, int errnum, const char *message, - va_list args, unsigned int mode_flags); + va_list args, unsigned int mode_flags) __COLD; void __error_at_line_internal (int status, int errnum, const char *file_name, unsigned int line_number, const char *message, - va_list args, unsigned int mode_flags); + va_list args, unsigned int mode_flags) __COLD; #endif diff --git a/libio/stdio.h b/libio/stdio.h index 45ddafdf..2387590d 100644 --- a/libio/stdio.h +++ b/libio/stdio.h @@ -859,7 +859,7 @@ extern int ferror_unlocked (FILE *__stream) __THROW __wur; This function is a possible cancellation point and therefore not marked with __THROW. */ -extern void perror (const char *__s); +extern void perror (const char *__s) __COLD; #ifdef __USE_POSIX diff --git a/misc/err.h b/misc/err.h index 0c752465..43df3a57 100644 --- a/misc/err.h +++ b/misc/err.h @@ -32,9 +32,9 @@ __BEGIN_DECLS /* Print "program: ", FORMAT, ": ", the standard error string for errno, and a newline, on stderr. */ extern void warn (const char *__format, ...) - __attribute__ ((__format__ (__printf__, 1, 2))); + __attribute__ ((__format__ (__printf__, 1, 2))) __COLD; extern void vwarn (const char *__format, __gnuc_va_list) - __attribute__ ((__format__ (__printf__, 1, 0))); + __attribute__ ((__format__ (__printf__, 1, 0))) __COLD; /* Likewise, but without ": " and the standard error string. */ extern void warnx (const char *__format, ...) @@ -44,13 +44,13 @@ extern void vwarnx (const char *__format, __gnuc_va_list) /* Likewise, and then exit with STATUS. */ extern void err (int __status, const char *__format, ...) - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; extern void verr (int __status, const char *__format, __gnuc_va_list) - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; extern void errx (int __status, const char *__format, ...) - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; extern void verrx (int __status, const char *, __gnuc_va_list) - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; #include <bits/floatn.h> #if defined __LDBL_COMPAT || __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 diff --git a/misc/error.h b/misc/error.h index 185b39c6..4fbd46c7 100644 --- a/misc/error.h +++ b/misc/error.h @@ -29,11 +29,11 @@ __BEGIN_DECLS If STATUS is nonzero, terminate the program with `exit (STATUS)'. */ extern void error (int __status, int __errnum, const char *__format, ...) - __attribute__ ((__format__ (__printf__, 3, 4))); + __attribute__ ((__format__ (__printf__, 3, 4))) __COLD; extern void error_at_line (int __status, int __errnum, const char *__fname, unsigned int __lineno, const char *__format, ...) - __attribute__ ((__format__ (__printf__, 5, 6))); + __attribute__ ((__format__ (__printf__, 5, 6))) __COLD; /* If NULL, error will flush stdout, then print on stderr the program name, a colon and a space. Otherwise, error will call this -- 2.40.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Mark various cold functions as __COLD 2023-04-29 13:12 ` [PATCH 3/3] Mark various cold " Sergey Bugaev @ 2023-04-29 15:06 ` Samuel Thibault 2023-05-01 16:15 ` Siddhesh Poyarekar 2023-05-01 21:52 ` Cristian Rodríguez 1 sibling, 1 reply; 12+ messages in thread From: Samuel Thibault @ 2023-04-29 15:06 UTC (permalink / raw) To: Sergey Bugaev; +Cc: libc-alpha, bug-hurd It looks sensible to me indeed, any opinion from somebody else? Sergey Bugaev, le sam. 29 avril 2023 16:12:23 +0300, a ecrit: > GCC docs explicitly list perror () as a good candidate for using > __attribute__ ((cold)). So apply __COLD to perror () and similar > functions. > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > include/error.h | 4 ++-- > libio/stdio.h | 2 +- > misc/err.h | 12 ++++++------ > misc/error.h | 4 ++-- > 4 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/include/error.h b/include/error.h > index 9e96262f..4db67cba 100644 > --- a/include/error.h > +++ b/include/error.h > @@ -5,11 +5,11 @@ > > void > __error_internal (int status, int errnum, const char *message, > - va_list args, unsigned int mode_flags); > + va_list args, unsigned int mode_flags) __COLD; > > void > __error_at_line_internal (int status, int errnum, const char *file_name, > unsigned int line_number, const char *message, > - va_list args, unsigned int mode_flags); > + va_list args, unsigned int mode_flags) __COLD; > > #endif > diff --git a/libio/stdio.h b/libio/stdio.h > index 45ddafdf..2387590d 100644 > --- a/libio/stdio.h > +++ b/libio/stdio.h > @@ -859,7 +859,7 @@ extern int ferror_unlocked (FILE *__stream) __THROW __wur; > > This function is a possible cancellation point and therefore not > marked with __THROW. */ > -extern void perror (const char *__s); > +extern void perror (const char *__s) __COLD; > > > #ifdef __USE_POSIX > diff --git a/misc/err.h b/misc/err.h > index 0c752465..43df3a57 100644 > --- a/misc/err.h > +++ b/misc/err.h > @@ -32,9 +32,9 @@ __BEGIN_DECLS > /* Print "program: ", FORMAT, ": ", the standard error string for errno, > and a newline, on stderr. */ > extern void warn (const char *__format, ...) > - __attribute__ ((__format__ (__printf__, 1, 2))); > + __attribute__ ((__format__ (__printf__, 1, 2))) __COLD; > extern void vwarn (const char *__format, __gnuc_va_list) > - __attribute__ ((__format__ (__printf__, 1, 0))); > + __attribute__ ((__format__ (__printf__, 1, 0))) __COLD; > > /* Likewise, but without ": " and the standard error string. */ > extern void warnx (const char *__format, ...) > @@ -44,13 +44,13 @@ extern void vwarnx (const char *__format, __gnuc_va_list) > > /* Likewise, and then exit with STATUS. */ > extern void err (int __status, const char *__format, ...) > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; > extern void verr (int __status, const char *__format, __gnuc_va_list) > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; > extern void errx (int __status, const char *__format, ...) > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; > extern void verrx (int __status, const char *, __gnuc_va_list) > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; > > #include <bits/floatn.h> > #if defined __LDBL_COMPAT || __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 > diff --git a/misc/error.h b/misc/error.h > index 185b39c6..4fbd46c7 100644 > --- a/misc/error.h > +++ b/misc/error.h > @@ -29,11 +29,11 @@ __BEGIN_DECLS > If STATUS is nonzero, terminate the program with `exit (STATUS)'. */ > > extern void error (int __status, int __errnum, const char *__format, ...) > - __attribute__ ((__format__ (__printf__, 3, 4))); > + __attribute__ ((__format__ (__printf__, 3, 4))) __COLD; > > extern void error_at_line (int __status, int __errnum, const char *__fname, > unsigned int __lineno, const char *__format, ...) > - __attribute__ ((__format__ (__printf__, 5, 6))); > + __attribute__ ((__format__ (__printf__, 5, 6))) __COLD; > > /* If NULL, error will flush stdout, then print on stderr the program > name, a colon and a space. Otherwise, error will call this > -- > 2.40.1 > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Mark various cold functions as __COLD 2023-04-29 15:06 ` Samuel Thibault @ 2023-05-01 16:15 ` Siddhesh Poyarekar 2023-05-01 17:38 ` Samuel Thibault 0 siblings, 1 reply; 12+ messages in thread From: Siddhesh Poyarekar @ 2023-05-01 16:15 UTC (permalink / raw) To: Sergey Bugaev, libc-alpha, bug-hurd On 2023-04-29 11:06, Samuel Thibault via Libc-alpha wrote: > It looks sensible to me indeed, any opinion from somebody else? > > > > > Sergey Bugaev, le sam. 29 avril 2023 16:12:23 +0300, a ecrit: >> GCC docs explicitly list perror () as a good candidate for using >> __attribute__ ((cold)). So apply __COLD to perror () and similar >> functions. >> >> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> LGTM. It would be nice to do a build-many-glibcs run here since it touches public headers but the change itself looks fairly straightforward and should be fine for all compilers. Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org> >> --- >> include/error.h | 4 ++-- >> libio/stdio.h | 2 +- >> misc/err.h | 12 ++++++------ >> misc/error.h | 4 ++-- >> 4 files changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/include/error.h b/include/error.h >> index 9e96262f..4db67cba 100644 >> --- a/include/error.h >> +++ b/include/error.h >> @@ -5,11 +5,11 @@ >> >> void >> __error_internal (int status, int errnum, const char *message, >> - va_list args, unsigned int mode_flags); >> + va_list args, unsigned int mode_flags) __COLD; >> >> void >> __error_at_line_internal (int status, int errnum, const char *file_name, >> unsigned int line_number, const char *message, >> - va_list args, unsigned int mode_flags); >> + va_list args, unsigned int mode_flags) __COLD; >> >> #endif >> diff --git a/libio/stdio.h b/libio/stdio.h >> index 45ddafdf..2387590d 100644 >> --- a/libio/stdio.h >> +++ b/libio/stdio.h >> @@ -859,7 +859,7 @@ extern int ferror_unlocked (FILE *__stream) __THROW __wur; >> >> This function is a possible cancellation point and therefore not >> marked with __THROW. */ >> -extern void perror (const char *__s); >> +extern void perror (const char *__s) __COLD; >> >> >> #ifdef __USE_POSIX >> diff --git a/misc/err.h b/misc/err.h >> index 0c752465..43df3a57 100644 >> --- a/misc/err.h >> +++ b/misc/err.h >> @@ -32,9 +32,9 @@ __BEGIN_DECLS >> /* Print "program: ", FORMAT, ": ", the standard error string for errno, >> and a newline, on stderr. */ >> extern void warn (const char *__format, ...) >> - __attribute__ ((__format__ (__printf__, 1, 2))); >> + __attribute__ ((__format__ (__printf__, 1, 2))) __COLD; >> extern void vwarn (const char *__format, __gnuc_va_list) >> - __attribute__ ((__format__ (__printf__, 1, 0))); >> + __attribute__ ((__format__ (__printf__, 1, 0))) __COLD; >> >> /* Likewise, but without ": " and the standard error string. */ >> extern void warnx (const char *__format, ...) >> @@ -44,13 +44,13 @@ extern void vwarnx (const char *__format, __gnuc_va_list) >> >> /* Likewise, and then exit with STATUS. */ >> extern void err (int __status, const char *__format, ...) >> - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); >> + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; >> extern void verr (int __status, const char *__format, __gnuc_va_list) >> - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); >> + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; >> extern void errx (int __status, const char *__format, ...) >> - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); >> + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; >> extern void verrx (int __status, const char *, __gnuc_va_list) >> - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); >> + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; >> >> #include <bits/floatn.h> >> #if defined __LDBL_COMPAT || __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 >> diff --git a/misc/error.h b/misc/error.h >> index 185b39c6..4fbd46c7 100644 >> --- a/misc/error.h >> +++ b/misc/error.h >> @@ -29,11 +29,11 @@ __BEGIN_DECLS >> If STATUS is nonzero, terminate the program with `exit (STATUS)'. */ >> >> extern void error (int __status, int __errnum, const char *__format, ...) >> - __attribute__ ((__format__ (__printf__, 3, 4))); >> + __attribute__ ((__format__ (__printf__, 3, 4))) __COLD; >> >> extern void error_at_line (int __status, int __errnum, const char *__fname, >> unsigned int __lineno, const char *__format, ...) >> - __attribute__ ((__format__ (__printf__, 5, 6))); >> + __attribute__ ((__format__ (__printf__, 5, 6))) __COLD; >> >> /* If NULL, error will flush stdout, then print on stderr the program >> name, a colon and a space. Otherwise, error will call this >> -- >> 2.40.1 >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Mark various cold functions as __COLD 2023-05-01 16:15 ` Siddhesh Poyarekar @ 2023-05-01 17:38 ` Samuel Thibault 0 siblings, 0 replies; 12+ messages in thread From: Samuel Thibault @ 2023-05-01 17:38 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: Sergey Bugaev, libc-alpha, bug-hurd Siddhesh Poyarekar, le lun. 01 mai 2023 12:15:04 -0400, a ecrit: > > > On 2023-04-29 11:06, Samuel Thibault via Libc-alpha wrote: > > It looks sensible to me indeed, any opinion from somebody else? > > > > > > > > > > Sergey Bugaev, le sam. 29 avril 2023 16:12:23 +0300, a ecrit: > > > GCC docs explicitly list perror () as a good candidate for using > > > __attribute__ ((cold)). So apply __COLD to perror () and similar > > > functions. > > > > > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > > LGTM. It would be nice to do a build-many-glibcs run here since it > touches public headers Yes, I ran i686-gnu, x86_64-gnu and x86_64-linux-gnu. > but the change itself looks fairly straightforward and should > be fine for all compilers. Applied, thanks! > Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > > > > --- > > > include/error.h | 4 ++-- > > > libio/stdio.h | 2 +- > > > misc/err.h | 12 ++++++------ > > > misc/error.h | 4 ++-- > > > 4 files changed, 11 insertions(+), 11 deletions(-) > > > > > > diff --git a/include/error.h b/include/error.h > > > index 9e96262f..4db67cba 100644 > > > --- a/include/error.h > > > +++ b/include/error.h > > > @@ -5,11 +5,11 @@ > > > void > > > __error_internal (int status, int errnum, const char *message, > > > - va_list args, unsigned int mode_flags); > > > + va_list args, unsigned int mode_flags) __COLD; > > > void > > > __error_at_line_internal (int status, int errnum, const char *file_name, > > > unsigned int line_number, const char *message, > > > - va_list args, unsigned int mode_flags); > > > + va_list args, unsigned int mode_flags) __COLD; > > > #endif > > > diff --git a/libio/stdio.h b/libio/stdio.h > > > index 45ddafdf..2387590d 100644 > > > --- a/libio/stdio.h > > > +++ b/libio/stdio.h > > > @@ -859,7 +859,7 @@ extern int ferror_unlocked (FILE *__stream) __THROW __wur; > > > This function is a possible cancellation point and therefore not > > > marked with __THROW. */ > > > -extern void perror (const char *__s); > > > +extern void perror (const char *__s) __COLD; > > > #ifdef __USE_POSIX > > > diff --git a/misc/err.h b/misc/err.h > > > index 0c752465..43df3a57 100644 > > > --- a/misc/err.h > > > +++ b/misc/err.h > > > @@ -32,9 +32,9 @@ __BEGIN_DECLS > > > /* Print "program: ", FORMAT, ": ", the standard error string for errno, > > > and a newline, on stderr. */ > > > extern void warn (const char *__format, ...) > > > - __attribute__ ((__format__ (__printf__, 1, 2))); > > > + __attribute__ ((__format__ (__printf__, 1, 2))) __COLD; > > > extern void vwarn (const char *__format, __gnuc_va_list) > > > - __attribute__ ((__format__ (__printf__, 1, 0))); > > > + __attribute__ ((__format__ (__printf__, 1, 0))) __COLD; > > > /* Likewise, but without ": " and the standard error string. */ > > > extern void warnx (const char *__format, ...) > > > @@ -44,13 +44,13 @@ extern void vwarnx (const char *__format, __gnuc_va_list) > > > /* Likewise, and then exit with STATUS. */ > > > extern void err (int __status, const char *__format, ...) > > > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); > > > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; > > > extern void verr (int __status, const char *__format, __gnuc_va_list) > > > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); > > > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; > > > extern void errx (int __status, const char *__format, ...) > > > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))); > > > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 3))) __COLD; > > > extern void verrx (int __status, const char *, __gnuc_va_list) > > > - __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))); > > > + __attribute__ ((__noreturn__, __format__ (__printf__, 2, 0))) __COLD; > > > #include <bits/floatn.h> > > > #if defined __LDBL_COMPAT || __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 > > > diff --git a/misc/error.h b/misc/error.h > > > index 185b39c6..4fbd46c7 100644 > > > --- a/misc/error.h > > > +++ b/misc/error.h > > > @@ -29,11 +29,11 @@ __BEGIN_DECLS > > > If STATUS is nonzero, terminate the program with `exit (STATUS)'. */ > > > extern void error (int __status, int __errnum, const char *__format, ...) > > > - __attribute__ ((__format__ (__printf__, 3, 4))); > > > + __attribute__ ((__format__ (__printf__, 3, 4))) __COLD; > > > extern void error_at_line (int __status, int __errnum, const char *__fname, > > > unsigned int __lineno, const char *__format, ...) > > > - __attribute__ ((__format__ (__printf__, 5, 6))); > > > + __attribute__ ((__format__ (__printf__, 5, 6))) __COLD; > > > /* If NULL, error will flush stdout, then print on stderr the program > > > name, a colon and a space. Otherwise, error will call this > > > -- > > > 2.40.1 > > > > > > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Mark various cold functions as __COLD 2023-04-29 13:12 ` [PATCH 3/3] Mark various cold " Sergey Bugaev 2023-04-29 15:06 ` Samuel Thibault @ 2023-05-01 21:52 ` Cristian Rodríguez 2023-05-01 21:54 ` Samuel Thibault 1 sibling, 1 reply; 12+ messages in thread From: Cristian Rodríguez @ 2023-05-01 21:52 UTC (permalink / raw) To: Sergey Bugaev; +Cc: libc-alpha, bug-hurd, Samuel Thibault [-- Attachment #1: Type: text/plain, Size: 362 bytes --] a On Sat, Apr 29, 2023 at 9:12 AM Sergey Bugaev via Libc-alpha < libc-alpha@sourceware.org> wrote: > GCC docs explicitly list perror () as a good candidate for using > __attribute__ ((cold)). So apply __COLD to perror () and similar > functions. > abort() and all the function wrappers called by assert on failure are also cold. Cheers! > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Mark various cold functions as __COLD 2023-05-01 21:52 ` Cristian Rodríguez @ 2023-05-01 21:54 ` Samuel Thibault 2023-05-02 6:45 ` Sergey Bugaev 0 siblings, 1 reply; 12+ messages in thread From: Samuel Thibault @ 2023-05-01 21:54 UTC (permalink / raw) To: Cristian Rodríguez; +Cc: Sergey Bugaev, libc-alpha, bug-hurd Cristian Rodríguez, le lun. 01 mai 2023 17:52:17 -0400, a ecrit: > On Sat, Apr 29, 2023 at 9:12 AM Sergey Bugaev via Libc-alpha <[1] > libc-alpha@sourceware.org> wrote: > > GCC docs explicitly list perror () as a good candidate for using > __attribute__ ((cold)). So apply __COLD to perror () and similar > functions. > > > abort() and all the function wrappers called by assert on failure are also > cold. Indeed, most __noreturn__ functions a probably worth having a look at. Samuel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] Mark various cold functions as __COLD 2023-05-01 21:54 ` Samuel Thibault @ 2023-05-02 6:45 ` Sergey Bugaev 0 siblings, 0 replies; 12+ messages in thread From: Sergey Bugaev @ 2023-05-02 6:45 UTC (permalink / raw) To: Cristian Rodríguez, Samuel Thibault; +Cc: libc-alpha Hello, On Tue, May 2, 2023 at 12:54 AM Samuel Thibault <samuel.thibault@gnu.org> wrote: > Cristian Rodríguez, le lun. 01 mai 2023 17:52:17 -0400, a ecrit: > > abort() and all the function wrappers called by assert on failure are also > > cold. > > Indeed, most __noreturn__ functions a probably worth having a look at. I was under the impression that noreturn functions automatically get treated similarly to ones with an explicit attribute((cold)). (But I still marked some, but not all -- fairly arbitrarily -- noreturn functions __COLD in this patch...) From taking a closer look now, I see that while the generated assembly instructions may be similar/identical, it's not strictly the same thing: with attribute((cold)) the cold path code goes into .text.unlikely, and without it's still in the regular .text. So applying __COLD should be beneficial indeed. I will send a follow-up patch; thank you for pointing this out and making me double-check :) Is there some sort of a benchmark where we could see whether this makes any difference? (Not calling the cold function in a loop, of course -- that is likely to regress, in fact -- but rather calling a hot function in a loop and benefitting from the cold path not getting in the way, improved code locality etc.) Sergey ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] cdefs.h: Define __COLD 2023-04-29 13:12 [PATCH 1/3] cdefs.h: Define __COLD Sergey Bugaev 2023-04-29 13:12 ` [PATCH 2/3] hurd: Mark error functions as __COLD Sergey Bugaev 2023-04-29 13:12 ` [PATCH 3/3] Mark various cold " Sergey Bugaev @ 2023-04-29 13:20 ` Sergey Bugaev 2023-04-29 15:02 ` Samuel Thibault 2 siblings, 1 reply; 12+ messages in thread From: Sergey Bugaev @ 2023-04-29 13:20 UTC (permalink / raw) To: libc-alpha; +Cc: bug-hurd, Samuel Thibault And of course right after I have sent it, I notice that I forgot the "Not GCC or clang." case. Sergey -- >8 -- From 1b8c6563828399de563846525d0f525001f2d80d Mon Sep 17 00:00:00 2001 From: Sergey Bugaev <bugaevc@gmail.com> Date: Thu, 27 Apr 2023 17:42:11 +0300 Subject: [PATCH] cdefs.h: Define __COLD This expands to __attribute__ ((cold)) when supported. It should be used to mark up functions that are invoked rarely. Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- misc/sys/cdefs.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index 23ec0ebd..9a07e297 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -98,6 +98,12 @@ # endif # endif +# if __GNUC_PREREQ (4, 3) || __glibc_has_attribute (__cold__) +# define __COLD __attribute__ ((__cold__)) +# else +# define __COLD +# endif + #else /* Not GCC or clang. */ # if (defined __cplusplus \ @@ -110,6 +116,7 @@ # define __THROW # define __THROWNL # define __NTH(fct) fct +# define __COLD #endif /* GCC || clang. */ -- 2.40.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] cdefs.h: Define __COLD 2023-04-29 13:20 ` [PATCH] cdefs.h: Define __COLD Sergey Bugaev @ 2023-04-29 15:02 ` Samuel Thibault 0 siblings, 0 replies; 12+ messages in thread From: Samuel Thibault @ 2023-04-29 15:02 UTC (permalink / raw) To: Sergey Bugaev; +Cc: libc-alpha, bug-hurd Applied, thanks! Sergey Bugaev, le sam. 29 avril 2023 16:20:53 +0300, a ecrit: > And of course right after I have sent it, I notice that I forgot the > "Not GCC or clang." case. > > Sergey > > -- >8 -- > > From 1b8c6563828399de563846525d0f525001f2d80d Mon Sep 17 00:00:00 2001 > From: Sergey Bugaev <bugaevc@gmail.com> > Date: Thu, 27 Apr 2023 17:42:11 +0300 > Subject: [PATCH] cdefs.h: Define __COLD > > This expands to __attribute__ ((cold)) when supported. It should be > used to mark up functions that are invoked rarely. > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > misc/sys/cdefs.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h > index 23ec0ebd..9a07e297 100644 > --- a/misc/sys/cdefs.h > +++ b/misc/sys/cdefs.h > @@ -98,6 +98,12 @@ > # endif > # endif > > +# if __GNUC_PREREQ (4, 3) || __glibc_has_attribute (__cold__) > +# define __COLD __attribute__ ((__cold__)) > +# else > +# define __COLD > +# endif > + > #else /* Not GCC or clang. */ > > # if (defined __cplusplus \ > @@ -110,6 +116,7 @@ > # define __THROW > # define __THROWNL > # define __NTH(fct) fct > +# define __COLD > > #endif /* GCC || clang. */ > > -- > 2.40.1 > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-05-02 6:45 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-29 13:12 [PATCH 1/3] cdefs.h: Define __COLD Sergey Bugaev 2023-04-29 13:12 ` [PATCH 2/3] hurd: Mark error functions as __COLD Sergey Bugaev 2023-04-29 15:04 ` Samuel Thibault 2023-04-29 13:12 ` [PATCH 3/3] Mark various cold " Sergey Bugaev 2023-04-29 15:06 ` Samuel Thibault 2023-05-01 16:15 ` Siddhesh Poyarekar 2023-05-01 17:38 ` Samuel Thibault 2023-05-01 21:52 ` Cristian Rodríguez 2023-05-01 21:54 ` Samuel Thibault 2023-05-02 6:45 ` Sergey Bugaev 2023-04-29 13:20 ` [PATCH] cdefs.h: Define __COLD Sergey Bugaev 2023-04-29 15:02 ` Samuel Thibault
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).