* [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
* [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] 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
* 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
* 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
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).