assert: Wrap __assert_fail in templated function

The idea of this commit to essentially save code side in the "hot"
`assert` passes case.

By wrapping `__assert_fail` with a templated function and using the
quasi unique `__LINE__` constant as the template parameter, we
essentially ensure that each `assert` gets forwarded through a unique
function. This then allows any reasonable optimizing compiler the
clone the wrapper function and constant propagate the arguments passed
(starts at -O2 for GCC/Clang). The result of the seperate, is that the
codegen for setting up `__expression`, `__line`, `__file`, and
`__functions` for the function call are moved to the presumed cold
code on the assertion failure path. Having a seperate cold function
allows the compiler/linker to move assertion setup code to a cold
section ultimately saving space in potentially hot regions.

See examples: https://godbolt.org/z/vMvrjnvaE
---
 assert/assert.h | 117 ++++++++++++++++++++++++++++++------------------
 1 file changed, 74 insertions(+), 43 deletions(-)

On 22/07/24 14:43, Noah Goldstein wrote:
> The idea of this commit to essentially save code side in the "hot"
> `assert` passes case.
> 
> By wrapping `__assert_fail` with a templated function and using the
> quasi unique `__LINE__` constant as the template parameter, we
> essentially ensure that each `assert` gets forwarded through a unique
> function. This then allows any reasonable optimizing compiler the
> clone the wrapper function and constant propagate the arguments passed
> (starts at -O2 for GCC/Clang). The result of the seperate, is that the
> codegen for setting up `__expression`, `__line`, `__file`, and
> `__functions` for the function call are moved to the presumed cold
> code on the assertion failure path. Having a seperate cold function
> allows the compiler/linker to move assertion setup code to a cold
> section ultimately saving space in potentially hot regions.
> 
> See examples: https://godbolt.org/z/vMvrjnvaE

It seems ok, have you tested this change against a large C++ program like
gcc or clang (with assert enabled) to see if there is any meaningful change?

> ---
>  assert/assert.h | 117 ++++++++++++++++++++++++++++++------------------
>  1 file changed, 74 insertions(+), 43 deletions(-)
> 
> diff --git a/assert/assert.h b/assert/assert.h
> index 266a41df06..ea8dcdb8cd 100644
> --- a/assert/assert.h
> +++ b/assert/assert.h
> @@ -82,66 +82,97 @@ extern void __assert (const char *__assertion, const char *__file, int __line)
>  
>  __END_DECLS
>  
> +# if defined __cplusplus && __cplusplus >= 201103L                           \
> +      && (defined __GNUC__ || defined __clang__)
> +template <unsigned int __unused>
> +static __attribute__ ((__noreturn__)) __attribute_noinline__ __COLD void
> +__assert_fail_forwarder (const char *__assertion, const char *__file,
> +			 unsigned int __line, const char *__function)
> +{
> +  __assert_fail (__assertion, __file, __line, __function);
> +}
> +#  define __ASSERT_FAIL(expr, file, line, func)                             \
> +      __assert_fail_forwarder<line> (expr, file, line, func)
> +
> +#  ifdef __USE_GNU
> +template <unsigned int __unused>
> +static __attribute__ ((__noreturn__)) __attribute_noinline__ __COLD void
> +__assert_perror_fail_forwarder (int __errnum, const char *__file,
> +				unsigned int __line, const char *__function)
> +{
> +  __assert_perror_fail (__errnum, __file, __line, __function);
> +}
> +#   define __ASSERT_PERROR_FAIL(errnum, file, line, func)                  \
> +	__assert_perror_fail_forwarder<line> (errnum, file, line, func)
> +#  endif
> +# else
> +#  define __ASSERT_FAIL(expr, file, line, func)                             \
> +      __assert_fail (expr, file, line, func)
> +#  ifdef __USE_GNU
> +#   define __ASSERT_PERROR_FAIL(errnum, file, line, func)                  \
> +	__assert_perror_fail (errnum, file, line, func)
> +#  endif
> +# endif
> +
>  /* When possible, define assert so that it does not add extra
>     parentheses around EXPR.  Otherwise, those added parentheses would
>     suppress warnings we'd expect to be detected by gcc's -Wparentheses.  */
> -# if defined __cplusplus
> -#  if defined __has_builtin
> -#   if __has_builtin (__builtin_FILE)
> -#    define __ASSERT_FILE __builtin_FILE ()
> -#    define __ASSERT_LINE __builtin_LINE ()
> -#   endif
> -#  endif
> -#  if !defined __ASSERT_FILE
> -#   define __ASSERT_FILE __FILE__
> -#   define __ASSERT_LINE __LINE__
> -#  endif
> -#  define assert(expr)							\
> -     (static_cast <bool> (expr)						\
> -      ? void (0)							\
> -      : __assert_fail (#expr, __ASSERT_FILE, __ASSERT_LINE,             \
> -                       __ASSERT_FUNCTION))
> -# elif !defined __GNUC__ || defined __STRICT_ANSI__
> -#  define assert(expr)							\
> -    ((expr)								\
> -     ? __ASSERT_VOID_CAST (0)						\
> -     : __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION))
> -# else
> +#  if defined __cplusplus
> +#    if defined __has_builtin
> +#      if __has_builtin(__builtin_FILE)
> +#	define __ASSERT_FILE __builtin_FILE ()
> +#	define __ASSERT_LINE __builtin_LINE ()
> +#      endif
> +#    endif
> +#    if !defined __ASSERT_FILE
> +#      define __ASSERT_FILE __FILE__
> +#      define __ASSERT_LINE __LINE__
> +#    endif
> +#    define assert(expr)                                                      \
> +      (static_cast<bool> (expr)                                               \
> +	   ? void (0)                                                         \
> +	   : __ASSERT_FAIL (#expr, __ASSERT_FILE, __ASSERT_LINE,              \
> +			    __ASSERT_FUNCTION))
> +#  elif !defined __GNUC__ || defined __STRICT_ANSI__
> +#    define assert(expr)                                                      \
> +      ((expr) ? __ASSERT_VOID_CAST (0)                                        \
> +	      : __ASSERT_FAIL (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION))
> +#  else
>  /* The first occurrence of EXPR is not evaluated due to the sizeof,
>     but will trigger any pedantic warnings masked by the __extension__
>     for the second occurrence.  The ternary operator is required to
>     support function pointers and bit fields in this context, and to
>     suppress the evaluation of variable length arrays.  */
> -#  define assert(expr)							\
> -  ((void) sizeof ((expr) ? 1 : 0), __extension__ ({			\
> -      if (expr)								\
> -        ; /* empty */							\
> -      else								\
> -        __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION);	\
> -    }))
> -# endif
> +#    define assert(expr)                                                      \
> +      ((void) sizeof ((expr) ? 1 : 0), __extension__ ({                       \
> +	 if (expr)                                                            \
> +	   ; /* empty */                                                      \
> +	 else                                                                 \
> +	   __ASSERT_FAIL (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION);      \
> +       }))
> +#  endif
>  
> -# ifdef	__USE_GNU
> -#  define assert_perror(errnum)						\
> -  (!(errnum)								\
> -   ? __ASSERT_VOID_CAST (0)						\
> -   : __assert_perror_fail ((errnum), __FILE__, __LINE__, __ASSERT_FUNCTION))
> -# endif
> +#  ifdef __USE_GNU
> +#    define assert_perror(errnum)                                             \
> +      (!(errnum) ? __ASSERT_VOID_CAST (0)                                     \
> +		 : __ASSERT_PERROR_FAIL ((errnum), __FILE__, __LINE__,        \
> +					 __ASSERT_FUNCTION))
> +#  endif
>  
>  /* Version 2.4 and later of GCC define a magical variable `__PRETTY_FUNCTION__'
>     which contains the name of the function currently being defined.
>     This is broken in G++ before version 2.6.
>     C9x has a similar variable called __func__, but prefer the GCC one since
>     it demangles C++ function names.  */
> -# if defined __cplusplus ? __GNUC_PREREQ (2, 6) : __GNUC_PREREQ (2, 4)
> -#   define __ASSERT_FUNCTION	__extension__ __PRETTY_FUNCTION__
> -# else
> -#  if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
> -#   define __ASSERT_FUNCTION	__func__
> +#  if defined __cplusplus ? __GNUC_PREREQ(2, 6) : __GNUC_PREREQ(2, 4)
> +#    define __ASSERT_FUNCTION __extension__ __PRETTY_FUNCTION__
>  #  else
> -#   define __ASSERT_FUNCTION	((const char *) 0)
> +#    if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
> +#      define __ASSERT_FUNCTION __func__
> +#    else
> +#      define __ASSERT_FUNCTION ((const char *) 0)
> +#    endif
>  #  endif
> -# endif
>  
>  #endif /* NDEBUG.  */
>

* Noah Goldstein:

> The idea of this commit to essentially save code side in the "hot"
> `assert` passes case.
>
> By wrapping `__assert_fail` with a templated function and using the
> quasi unique `__LINE__` constant as the template parameter, we
> essentially ensure that each `assert` gets forwarded through a unique
> function. This then allows any reasonable optimizing compiler the
> clone the wrapper function and constant propagate the arguments passed
> (starts at -O2 for GCC/Clang). The result of the seperate, is that the
> codegen for setting up `__expression`, `__line`, `__file`, and
> `__functions` for the function call are moved to the presumed cold
> code on the assertion failure path. Having a seperate cold function
> allows the compiler/linker to move assertion setup code to a cold
> section ultimately saving space in potentially hot regions.
>
> See examples: https://godbolt.org/z/vMvrjnvaE

Maybe the compiler can do this for us if we apply __COLD to
__assert_fail?

Thanks,
Florian

On Fri, Jul 26, 2024 at 3:22 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Noah Goldstein:
>
> > The idea of this commit to essentially save code side in the "hot"
> > `assert` passes case.
> >
> > By wrapping `__assert_fail` with a templated function and using the
> > quasi unique `__LINE__` constant as the template parameter, we
> > essentially ensure that each `assert` gets forwarded through a unique
> > function. This then allows any reasonable optimizing compiler the
> > clone the wrapper function and constant propagate the arguments passed
> > (starts at -O2 for GCC/Clang). The result of the seperate, is that the
> > codegen for setting up `__expression`, `__line`, `__file`, and
> > `__functions` for the function call are moved to the presumed cold
> > code on the assertion failure path. Having a seperate cold function
> > allows the compiler/linker to move assertion setup code to a cold
> > section ultimately saving space in potentially hot regions.
> >
> > See examples: https://godbolt.org/z/vMvrjnvaE
>
> Maybe the compiler can do this for us if we apply __COLD to
> __assert_fail?

https://godbolt.org/z/caox8YMY1

This works on GCC, I'll post an independent patch and rebase this one
one top.

It doesn't help LLVM, however. LLVM has `--hot-cold-splitting`
although at the moment it is regrettably unwilling fully de-optimize
`noreturn` paths because of `longjmp`.... I know right.
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/HotColdSplitting.cpp#L151

And since `__assert_fail` is non-standard (and not all libc implementations
even agree on its definition (i.e `musl` using a `signed int` for
line) it's hard
to recognize this one case.

If there were an attribute like `exits` that might be usable. But even
so `--hot-cold-splitting` is not on any default pipelines so its seems
unlikely that it will solve this problem anytime soon. (I am *slowly*
working on patches to handle this, but not anytime soon).

>
> Thanks,
> Florian
>