All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Heiko Carstens <hca@linux.ibm.com>
Cc: Miguel Ojeda <ojeda@kernel.org>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Alexander Gordeev <agordeev@linux.ibm.com>,
	Juergen Christ <jchrist@linux.ibm.com>,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	Sven Schnelle <svens@linux.ibm.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>
Subject: Re: [PATCH 1/3] Compiler Attributes: Add __assume macro
Date: Thu, 11 Sep 2025 11:44:29 -0700	[thread overview]
Message-ID: <20250911184429.GA2395987@ax162> (raw)
In-Reply-To: <20250911145659.8894Dea-hca@linux.ibm.com>

On Thu, Sep 11, 2025 at 04:56:59PM +0200, Heiko Carstens wrote:
> On Wed, Sep 10, 2025 at 06:32:43PM -0700, Nathan Chancellor wrote:
> > > + *
> > > + * Optional: only supported since GCC >= 13.1, clang >= 12.0
> > > + *
> > > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#index-assume-statement-attribute
> > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#assume
> > 
> > Looking at this link sent me down a bit of a rabbit hole :) Prior to
> > Clang 19.1.0 [2], assume was an OpenMP attribute, which has completely
> > different semantics and errors out when used in the way the series does:
> > 
> >   In file included from kernel/bounds.c:13:
> >   In file included from include/linux/log2.h:12:
> >   In file included from include/linux/bitops.h:67:
> >   arch/s390/include/asm/bitops.h:173:12: error: expected string literal as argument of '__assume__' attribute
> >     173 |                 __assume(bit <= 64);
> >         |                          ^
> > 
> > Unfortunately, I think __assume will need to be handled in the compiler
> > specific headers :/
> > 
> > [1]: https://clang.llvm.org/docs/AttributeReference.html#id13
> > [2]: https://github.com/llvm/llvm-project/commit/c44fa3e8a9a44c2e9a575768a3c185354b9f6c17
> 
> Thank you for having look. This is quite surprising. So after looking into the
> various header files it might be acceptable to add this to compiler_types.h,
> since there seem to be a few similar constructs.
> 
> Maybe something like this(?):

Ah, yeah, that would work too. I had not considered compiler_types.h
since most of those tend to involve dynamic checks via cc-option but I
do like keeping the documentation attached to the attribute in a single
location, rather than duplicating it in the individual compiler files.
This will also make it easy to move this into compiler_attributes.h in
the (likely distant) future when both compilers support this
unconditionally (or clang 19.1.0 is the minimum supported version so
__has_attribute can be used).

> From d9d67807e6854666507e55d9ac0c7b4ec659aa99 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <hca@linux.ibm.com>
> Date: Wed, 10 Sep 2025 14:18:07 +0200
> Subject: [PATCH] compiler_types: Add __assume macro
> 
> Make the statement attribute "assume" with a new __assume macro available.
> 
> This allows compilers to generate better code, however code which makes use
> of __assume must be written as if the compiler ignores the hint. Otherwise
> this may lead to subtle bugs if code is compiled with compilers which do
> not support the attribute.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

I do not think anyone really owns compiler_types.h so unless Miguel has
any objections from the compiler attributes perspective, I think you can
just take this via the s390 tree with the other two changes.

> ---
>  include/linux/compiler_types.h | 20 ++++++++++++++++++++
>  init/Kconfig                   | 10 ++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 16755431fc11..38a52a792e48 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -329,6 +329,26 @@ struct ftrace_likely_data {
>  #define __no_sanitize_or_inline __always_inline
>  #endif
>  
> +/*
> + * Beware: Code which makes use of __assume must be written as if the compiler
> + * ignores the hint. Otherwise this may lead to subtle bugs if code is compiled
> + * with compilers which do not support the attribute.
> + * Using this attribute requires careful analysis, since in some cases it may
> + * generate worse code (see clang documentation).
> + *
> + * Optional: only supported since gcc >= 13
> + * Optional: only supported since clang >= 19
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#index-assume-statement-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#id13
> + *
> + */
> +#ifdef CONFIG_CC_HAS_ASSUME
> +# define __assume(expr)			__attribute__((__assume__(expr)))
> +#else
> +# define __assume(expr)
> +#endif
> +
>  /*
>   * Optional: only supported since gcc >= 15
>   * Optional: only supported since clang >= 18
> diff --git a/init/Kconfig b/init/Kconfig
> index e3eb63eadc87..5882c5e74047 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -112,6 +112,16 @@ config TOOLS_SUPPORT_RELR
>  config CC_HAS_ASM_INLINE
>  	def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>  
> +config CC_HAS_ASSUME
> +	bool
> +	# clang needs to be at least 19.1.0 since the meaning of the assume
> +	# attribute changed:
> +	# https://github.com/llvm/llvm-project/commit/c44fa3e8a9a44c2e9a575768a3c185354b9f6c17
> +	default y if CC_IS_CLANG && CLANG_VERSION >= 190100
> +	# supported since gcc 13.1.0
> +	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106654
> +	default y if CC_IS_GCC && GCC_VERSION >= 130100
> +
>  config CC_HAS_NO_PROFILE_FN_ATTR
>  	def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
>  
> -- 
> 2.48.1

  reply	other threads:[~2025-09-11 18:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-10 15:12 [PATCH 0/3] s390: Fix and optimize __flogr() inline assembly Heiko Carstens
2025-09-10 15:12 ` [PATCH 1/3] Compiler Attributes: Add __assume macro Heiko Carstens
2025-09-11  1:32   ` Nathan Chancellor
2025-09-11 14:56     ` Heiko Carstens
2025-09-11 18:44       ` Nathan Chancellor [this message]
2025-09-11 19:04         ` Miguel Ojeda
2025-09-11 20:42           ` Nathan Chancellor
2025-09-11 18:56     ` Miguel Ojeda
2025-09-11 18:59   ` Miguel Ojeda
2025-09-12 10:25     ` Heiko Carstens
2025-09-10 15:12 ` [PATCH 2/3] s390/bitops: Limit return value range of __flogr() Heiko Carstens
2025-09-11  7:44   ` Juergen Christ
2025-09-11 13:24   ` kernel test robot
2025-09-10 15:12 ` [PATCH 3/3] s390/bitops: Remove volatile qualifier from flogr() inline assembly Heiko Carstens
2025-09-11  7:45   ` Juergen Christ

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250911184429.GA2395987@ax162 \
    --to=nathan@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=jchrist@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=svens@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.