| Message ID | 20240726084100.2582635-1-goldstein.w.n@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | assert: Mark `__assert_fail` as `cold` | expand |
* Noah Goldstein: > This helps compilers split the codegen for setting up the arguments > (`__expression`, `__filename`, etc...) from the potentially hot cold > where the `assert` is to a presumably cold region on the assertion > failure path. > --- > assert/assert.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/assert/assert.h b/assert/assert.h > index 266a41df06..3261eb4202 100644 > --- a/assert/assert.h > +++ b/assert/assert.h > @@ -71,13 +71,13 @@ extern void __assert_fail (const char *__assertion, const char *__file, > /* Likewise, but prints the error text for ERRNUM. */ > extern void __assert_perror_fail (int __errnum, const char *__file, > unsigned int __line, const char *__function) > - __THROW __attribute__ ((__noreturn__)); > + __THROW __attribute__ ((__noreturn__)) __COLD; > > > /* The following is not at all used here but needed for standard > compliance. */ > extern void __assert (const char *__assertion, const char *__file, int __line) > - __THROW __attribute__ ((__noreturn__)); > + __THROW __attribute__ ((__noreturn__)) __COLD; > > > __END_DECLS Looks okay. Reviewed-by: Florian Weimer <fweimer@redhat.com> Thanks, Florian
Noah Goldstein <goldstein.w.n@gmail.com> writes: > This helps compilers split the codegen for setting up the arguments > (`__expression`, `__filename`, etc...) from the potentially hot cold > where the `assert` is to a presumably cold region on the assertion > failure path. It looks good to me too, but I'm a little surprised this didn't come up before. I dug a bit more and found https://inbox.sourceware.org/libc-alpha/BYAPR08MB4232749003F118FA852BA0E5B6730@BYAPR08MB4232.namprd08.prod.outlook.com/, but nothing else. Reviewed-by: Sam James <sam@gentoo.org> > --- > assert/assert.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/assert/assert.h b/assert/assert.h > index 266a41df06..3261eb4202 100644 > --- a/assert/assert.h > +++ b/assert/assert.h > @@ -71,13 +71,13 @@ extern void __assert_fail (const char *__assertion, const char *__file, > /* Likewise, but prints the error text for ERRNUM. */ > extern void __assert_perror_fail (int __errnum, const char *__file, > unsigned int __line, const char *__function) > - __THROW __attribute__ ((__noreturn__)); > + __THROW __attribute__ ((__noreturn__)) __COLD; > > > /* The following is not at all used here but needed for standard > compliance. */ > extern void __assert (const char *__assertion, const char *__file, int __line) > - __THROW __attribute__ ((__noreturn__)); > + __THROW __attribute__ ((__noreturn__)) __COLD; > > > __END_DECLS
On Fri, 2024-07-26 at 16:41 +0800, Noah Goldstein wrote: > This helps compilers split the codegen for setting up the arguments > (`__expression`, `__filename`, etc...) from the potentially hot cold > where the `assert` is to a presumably cold region on the assertion > failure path. > --- > assert/assert.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/assert/assert.h b/assert/assert.h > index 266a41df06..3261eb4202 100644 > --- a/assert/assert.h > +++ b/assert/assert.h > @@ -71,13 +71,13 @@ extern void __assert_fail (const char *__assertion, const char *__file, > /* Likewise, but prints the error text for ERRNUM. */ > extern void __assert_perror_fail (int __errnum, const char *__file, > unsigned int __line, const char *__function) > - __THROW __attribute__ ((__noreturn__)); > + __THROW __attribute__ ((__noreturn__)) __COLD; Hmm, so "__assert_fail" isn't changed but "__assert_perror_fail" is changed despite the subject only mentions "__assert_fail"? Or am I missing something obvious here? > /* The following is not at all used here but needed for standard > compliance. */ > extern void __assert (const char *__assertion, const char *__file, int __line) > - __THROW __attribute__ ((__noreturn__)); > + __THROW __attribute__ ((__noreturn__)) __COLD; > > > __END_DECLS
On Fri, Jul 26, 2024 at 6:30 PM Xi Ruoyao <xry111@xry111.site> wrote: > > On Fri, 2024-07-26 at 16:41 +0800, Noah Goldstein wrote: > > This helps compilers split the codegen for setting up the arguments > > (`__expression`, `__filename`, etc...) from the potentially hot cold > > where the `assert` is to a presumably cold region on the assertion > > failure path. > > --- > > assert/assert.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/assert/assert.h b/assert/assert.h > > index 266a41df06..3261eb4202 100644 > > --- a/assert/assert.h > > +++ b/assert/assert.h > > @@ -71,13 +71,13 @@ extern void __assert_fail (const char *__assertion, const char *__file, > > /* Likewise, but prints the error text for ERRNUM. */ > > extern void __assert_perror_fail (int __errnum, const char *__file, > > unsigned int __line, const char *__function) > > - __THROW __attribute__ ((__noreturn__)); > > + __THROW __attribute__ ((__noreturn__)) __COLD; > > Hmm, so "__assert_fail" isn't changed but "__assert_perror_fail" is > changed despite the subject only mentions "__assert_fail"? > > Or am I missing something obvious here? > Nope, the patch didn't do the one thing it said it would xD Fixed in V2. > > /* The following is not at all used here but needed for standard > > compliance. */ > > extern void __assert (const char *__assertion, const char *__file, int __line) > > - __THROW __attribute__ ((__noreturn__)); > > + __THROW __attribute__ ((__noreturn__)) __COLD; > > > > > > __END_DECLS > > -- > Xi Ruoyao <xry111@xry111.site> > School of Aerospace Science and Technology, Xidian University
diff --git a/assert/assert.h b/assert/assert.h index 266a41df06..3261eb4202 100644 --- a/assert/assert.h +++ b/assert/assert.h @@ -71,13 +71,13 @@ extern void __assert_fail (const char *__assertion, const char *__file, /* Likewise, but prints the error text for ERRNUM. */ extern void __assert_perror_fail (int __errnum, const char *__file, unsigned int __line, const char *__function) - __THROW __attribute__ ((__noreturn__)); + __THROW __attribute__ ((__noreturn__)) __COLD; /* The following is not at all used here but needed for standard compliance. */ extern void __assert (const char *__assertion, const char *__file, int __line) - __THROW __attribute__ ((__noreturn__)); + __THROW __attribute__ ((__noreturn__)) __COLD; __END_DECLS