diff mbox series

[v2,01/15] cdefs: Add mechanism to add attributes to __always_inline functions

Message ID 20240527111900.1060546-2-christoph.muellner@vrull.eu (mailing list archive)
State New
Headers
Series RISC-V: Add Zbb-optimized string routines as ifuncs |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Christoph Müllner May 27, 2024, 11:18 a.m. UTC 
  Contrary to LLVM, GCC does not propagate function attributes accross
inlined functions ("target specific option mismatch").  To overcome this
limiation, this patch introduces a __CODEGEN_ATTRIBUTE macro, which can
be used to add function attributes to __always_inline functions.

Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
---
 misc/sys/cdefs.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Adhemerval Zanella Netto Aug. 13, 2024, 5:58 p.m. UTC | #1 
On 27/05/24 08:18, Christoph Müllner wrote:
> Contrary to LLVM, GCC does not propagate function attributes accross
> inlined functions ("target specific option mismatch").  To overcome this
> limiation, this patch introduces a __CODEGEN_ATTRIBUTE macro, which can
> be used to add function attributes to __always_inline functions.
> 
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>  misc/sys/cdefs.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index ab9620bd0d..91dceab1f2 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -587,15 +587,19 @@
>  #endif
>  
>  /* Forces a function to be always inlined.  */
> +# ifndef __CODEGEN_ATTRIBUTES
> +#  define __CODEGEN_ATTRIBUTES
> +# endif

So it seems to be required for the string-*.h headers to work correctly.
I though about either add a new arch-specific attribute on the string-*.h
header itself, but it seems that this is a compiler limitation itself and
tying to the __always_inline sounds reasonable.

The __CODEGEN_ATTRIBUTES name does not fit with the rest of the file,
I think something like __always_inline_codegen_attribute along with 
a brief explanation on why it should be required:

/* GCC requires to propagate the __attribute__ on static inline
   functions.  Append then on the inline macros if defined.  */
#ifndef __always_inline_codegen_attribute
# define __always_inline_codegen_attribute
#endif


>  #if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
>  /* The Linux kernel defines __always_inline in stddef.h (283d7573), and
>     it conflicts with this definition.  Therefore undefine it first to
>     allow either header to be included first.  */
>  # undef __always_inline
> -# define __always_inline __inline __attribute__ ((__always_inline__))
> +# define __always_inline __inline __attribute__ ((__always_inline__)) \
> +   __CODEGEN_ATTRIBUTES
>  #else
>  # undef __always_inline
> -# define __always_inline __inline
> +# define __always_inline __inline __CODEGEN_ATTRIBUTES
>  #endif
>  
>  /* Associate error messages with the source location of the call site rather
Florian Weimer Aug. 14, 2024, 9:29 a.m. UTC | #2 
* Christoph Müllner:

> Contrary to LLVM, GCC does not propagate function attributes accross
> inlined functions ("target specific option mismatch").  To overcome this
> limiation, this patch introduces a __CODEGEN_ATTRIBUTE macro, which can
> be used to add function attributes to __always_inline functions.
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>  misc/sys/cdefs.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index ab9620bd0d..91dceab1f2 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -587,15 +587,19 @@
>  #endif
>  
>  /* Forces a function to be always inlined.  */
> +# ifndef __CODEGEN_ATTRIBUTES
> +#  define __CODEGEN_ATTRIBUTES
> +# endif
>  #if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
>  /* The Linux kernel defines __always_inline in stddef.h (283d7573), and
>     it conflicts with this definition.  Therefore undefine it first to
>     allow either header to be included first.  */
>  # undef __always_inline
> -# define __always_inline __inline __attribute__ ((__always_inline__))
> +# define __always_inline __inline __attribute__ ((__always_inline__)) \
> +   __CODEGEN_ATTRIBUTES
>  #else
>  # undef __always_inline
> -# define __always_inline __inline
> +# define __always_inline __inline __CODEGEN_ATTRIBUTES
>  #endif
>  
>  /* Associate error messages with the source location of the call site rather

Is this required outside of glibc?  If not, it shouldn't be part of
<sys/cdefs.h>, which is an installed header.  Please add it to
include/sys/cdefs.h instead.

Thanks,
Florian
diff mbox series

Patch

diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index ab9620bd0d..91dceab1f2 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -587,15 +587,19 @@ 
 #endif
 
 /* Forces a function to be always inlined.  */
+# ifndef __CODEGEN_ATTRIBUTES
+#  define __CODEGEN_ATTRIBUTES
+# endif
 #if __GNUC_PREREQ (3,2) || __glibc_has_attribute (__always_inline__)
 /* The Linux kernel defines __always_inline in stddef.h (283d7573), and
    it conflicts with this definition.  Therefore undefine it first to
    allow either header to be included first.  */
 # undef __always_inline
-# define __always_inline __inline __attribute__ ((__always_inline__))
+# define __always_inline __inline __attribute__ ((__always_inline__)) \
+   __CODEGEN_ATTRIBUTES
 #else
 # undef __always_inline
-# define __always_inline __inline
+# define __always_inline __inline __CODEGEN_ATTRIBUTES
 #endif
 
 /* Associate error messages with the source location of the call site rather