[v1] stdlib: Mark `abort` as `cold`

This helps HotColdSplitting in GCC/LLVM.

Thought about doing `exit` as well since its only called once per
process, but since its easy to imagine a hot path leading into
`exit(0)`, its less clear if its profitable.
---
 stdlib/stdlib.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Mon, Jul 29, 2024 at 12:36 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> This helps HotColdSplitting in GCC/LLVM.
>
> Thought about doing `exit` as well since its only called once per
> process, but since its easy to imagine a hot path leading into
> `exit(0)`, its less clear if its profitable.

In fact GCC has predict heuristics explicitly to ignore `exit(0)` but
only for noreturn:
Added at https://gcc.gnu.org/r7-3919-ge25a366f6fbfec .

Thanks,
Andrew Pinski

> ---
>  stdlib/stdlib.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index 901926e893..17487c6640 100644
> --- a/stdlib/stdlib.h
> +++ b/stdlib/stdlib.h
> @@ -727,7 +727,7 @@ extern void *aligned_alloc (size_t __alignment, size_t __size)
>  #endif
>
>  /* Abort execution and generate a core-dump.  */
> -extern void abort (void) __THROW __attribute__ ((__noreturn__));
> +extern void abort (void) __THROW __attribute__ ((__noreturn__)) __COLD;
>
>
>  /* Register a function to be called when `exit' is called.  */
> --
> 2.34.1
>

On 2024-07-29 00:36, Noah Goldstein wrote:
> This helps HotColdSplitting in GCC/LLVM.

Why doesn't GCC/LLVM treat every noreturn function as cold? (They could 
make exceptions for exit (0) of course.)

And since 'abort' is already known by GCC/LLVM to be special, why can't 
GCC/LLVM optimize calls to 'abort' without glibc's help?

On Mon, Jul 29, 2024 at 4:04 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> On 2024-07-29 00:36, Noah Goldstein wrote:
> > This helps HotColdSplitting in GCC/LLVM.
>
> Why doesn't GCC/LLVM treat every noreturn function as cold? (They could
> make exceptions for exit (0) of course.)

At least LLVM doesn't handle the `noreturn` case b.c it could be something
like `longjmp`:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/HotColdSplitting.cpp#L151
>
> And since 'abort' is already known by GCC/LLVM to be special, why can't
> GCC/LLVM optimize calls to 'abort' without glibc's help?

GCC actually I think (I guess this really only helps LLVM), but either way
I don't see any downside to this...

On Mon, 2024-07-29 at 16:11 +0800, Noah Goldstein wrote:
> On Mon, Jul 29, 2024 at 4:04 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
> > 
> > On 2024-07-29 00:36, Noah Goldstein wrote:
> > > This helps HotColdSplitting in GCC/LLVM.
> > 
> > Why doesn't GCC/LLVM treat every noreturn function as cold? (They could
> > make exceptions for exit (0) of course.)
> 
> At least LLVM doesn't handle the `noreturn` case b.c it could be something
> like `longjmp`:
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/HotColdSplitting.cpp#L151
> > 
> > And since 'abort' is already known by GCC/LLVM to be special, why can't
> > GCC/LLVM optimize calls to 'abort' without glibc's help?
> 
> GCC actually I think (I guess this really only helps LLVM), but either way
> I don't see any downside to this...

A possible downside is it might be used as an excuse to post patches
adding "unlikely" and "likely" everywhere (even for obvious cases like
malloc failure for which every compiler should just do its own job w/o
any annotation; IIRC someone really attempted to add "unlikely" for
every malloc failure in Glibc once upon a time).

On Mon, Jul 29, 2024 at 4:23 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Mon, 2024-07-29 at 16:11 +0800, Noah Goldstein wrote:
> > On Mon, Jul 29, 2024 at 4:04 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
> > >
> > > On 2024-07-29 00:36, Noah Goldstein wrote:
> > > > This helps HotColdSplitting in GCC/LLVM.
> > >
> > > Why doesn't GCC/LLVM treat every noreturn function as cold? (They could
> > > make exceptions for exit (0) of course.)
> >
> > At least LLVM doesn't handle the `noreturn` case b.c it could be something
> > like `longjmp`:
> > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/HotColdSplitting.cpp#L151
> > >
> > > And since 'abort' is already known by GCC/LLVM to be special, why can't
> > > GCC/LLVM optimize calls to 'abort' without glibc's help?
> >
> > GCC actually I think (I guess this really only helps LLVM), but either way
> > I don't see any downside to this...
>
> A possible downside is it might be used as an excuse to post patches
> adding "unlikely" and "likely" everywhere (even for obvious cases like
> malloc failure for which every compiler should just do its own job w/o
> any annotation; IIRC someone really attempted to add "unlikely" for
> every malloc failure in Glibc once upon a time).

I don't think the malloc case is really equivalent.

For the malloc case, the alternative to implementing the logic in the
compiler is to use unlikely/likely at every malloc callsite. In this case
there is a fair case it's easier to just implement once in the compiler.

For `cold` attribute on a function it either needs to be just implemented
once in the compiler or once in libc. Between those to, putting it in the
source seems easier. It's the same reason we implement the rest of
the function attributes in source instead of the compile...

>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

On 7/29/24 3:36 AM, Noah Goldstein wrote:
> This helps HotColdSplitting in GCC/LLVM.
> 
> Thought about doing `exit` as well since its only called once per
> process, but since its easy to imagine a hot path leading into
> `exit(0)`, its less clear if its profitable.
> ---
>  stdlib/stdlib.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index 901926e893..17487c6640 100644
> --- a/stdlib/stdlib.h
> +++ b/stdlib/stdlib.h
> @@ -727,7 +727,7 @@ extern void *aligned_alloc (size_t __alignment, size_t __size)
>  #endif
>  
>  /* Abort execution and generate a core-dump.  */
> -extern void abort (void) __THROW __attribute__ ((__noreturn__));
> +extern void abort (void) __THROW __attribute__ ((__noreturn__)) __COLD;
>  
>  
>  /* Register a function to be called when `exit' is called.  */

I think this should just go in. It will take time to fix llvm and future compilers.

Marking it once here is practical.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

On Mon, Jul 29, 2024 at 3:36 AM Noah Goldstein <goldstein.w.n@gmail.com>
wrote:

> This helps HotColdSplitting in GCC/LLVM.
>
> Thought about doing `exit` as well since its only called once per
> process, but since its easy to imagine a hot path leading into
> `exit(0)`, its less clear if its profitable.
> ---
>  stdlib/stdlib.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index 901926e893..17487c6640 100644
> --- a/stdlib/stdlib.h
> +++ b/stdlib/stdlib.h
> @@ -727,7 +727,7 @@ extern void *aligned_alloc (size_t __alignment, size_t
> __size)
>  #endif
>
>  /* Abort execution and generate a core-dump.  */
> -extern void abort (void) __THROW __attribute__ ((__noreturn__));
> +extern void abort (void) __THROW __attribute__ ((__noreturn__)) __COLD;
>
>
>  /* Register a function to be called when `exit' is called.  */
> --
> 2.34.1
>
> GCC already uses cold on __builtin_abort so this patch is correct for the
cases where builtins are not used.

Same applies IMHO to strerror, strerror_r, strerror_l..

On Mon, Jul 29, 2024, at 11:10 AM, Cristian Rodríguez wrote:
> On Mon, Jul 29, 2024 at 3:36 AM Noah Goldstein
> <goldstein.w.n@gmail.com> wrote:
>> Thought about doing `exit` as well since its only called once per
>> process, but since its easy to imagine a hot path leading into
>> `exit(0)`, its less clear if its profitable.
>
> GCC already uses cold on __builtin_abort so this patch is correct for
> the cases where builtins are not used.
>
> Same applies IMHO to strerror, strerror_r, strerror_l..

strerror could easily wind up being called from inside a hot loop,
for instance reporting a whole lot of strtoul conversion failures.
I think it would be better to leave that family alone.

zw

On Mon, Jul 29, 2024 at 11:14 AM Zack Weinberg <zack@owlfolio.org> wrote:

>
> strerror could easily wind up being called from inside a hot loop,
> for instance reporting a whole lot of strtoul conversion failures.
> I think it would be better to leave that family alone.
>
> zw
>

I imagined such cases, yeah.unfortunately all I found was in the line of
if(some_error) printf the result of strerror and get out.