From: David Laight <david.laight.linux@gmail.com>
To: Matt Coster <matt.coster@imgtec.com>
Cc: Yury Norov <yury.norov@gmail.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Frank Binns <frank.binns@imgtec.com>,
Alessio Belle <alessio.belle@imgtec.com>,
Brajesh Gupta <brajesh.gupta@imgtec.com>,
Alexandru Dadu <alexandru.dadu@imgtec.com>,
<linux-kernel@vger.kernel.org>,
Vincent Mailhol <mailhol@kernel.org>,
kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] bitfield: Fix FIELD_PREP_CONST() with __GENMASK_ULL() on gcc<14
Date: Thu, 9 Apr 2026 19:48:33 +0100 [thread overview]
Message-ID: <20260409194833.3214139d@pumpkin> (raw)
In-Reply-To: <20260409-field-prep-fix-v1-1-f0e9ae64f63c@imgtec.com>
On Thu, 9 Apr 2026 15:57:53 +0100
Matt Coster <matt.coster@imgtec.com> wrote:
> There is a bug in gcc<14[1] that causes the following minimal example to
> not be treated as a constant:
>
> int main() {
> sizeof(struct {
> int t : !(__builtin_ffsll(~0ULL) + 1 < 0);
> });
> }
>
> test.c: In function 'main':
> test.c:3:21: error: bit-field 't' width not an integer constant
> 3 | int t : !(__builtin_ffsll(~0ULL) + 1 < 0);
> | ^
>
> The result of this bug is a bizarre interaction between FIELD_PREP_CONST()
> and __GENMASK_ULL(). Note that this does not occur with GENMASK_ULL() since
> that has not been based on the UAPI variant since commit 104ea1c84b91
> ("bits: unify the non-asm GENMASK*()").
Can you give the actual code that is failing.
I can only find the old version - that is pretty horrid itself.
It really isn't necessary to use GENMASK_ULL(15, 0) to get 0xffffu.
I've bumped into that file before.
It gave a false positive for FIELD_GET() returning a constant value
for non-constant input - which would be a nice check for invalid usage.
Someone went over the top on getting the pre-processor to bloat the
intermediate file by using the longest way possible to perform the
simplest of code patterns.
I suspect they've made it worse.
>
> The underlying compiler bug has been fixed in gcc since 14.1.0, but the fix
> appears to have been incidental in a larger change[2].
>
> [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124699
> [2]: https://gcc.gnu.org/cgit/gcc/commit/?id=0d00385eaf72ccacff17935b0d214a26773e095f
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202603222211.A2XiR1YU-lkp@intel.com/
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> ---
> Some below-the-dash thoughts:
>
> This is the most minimal workaround I could find. I'm not sure what a
> "real" fix would look like here so I'm open to suggestions.
Using a completely different expression instead of ffs() and shifts.
For constants it doesn't matter what you do.
Multiply/divide by mask & (~mask + 1) will be the same.
(I think it is used lower down the file.)
But passing the output of GENMASK() (any variant) into FIELD_PREP()
(any current variant) is a good way to generate pre-processor output
lines that are many kb long.
Oh, in my experiments, the value passed to ffsll() must be unsigned
and have the top bit set.
David
>
> The reproduction case is amazing because changing almost any subtle
> detail of it will result in the expression correctly being parsed as
> constant (even raising the construct to file scope; the use of
> BUILD_BUG_ON_ZERO() is part of the bizarre confluence required to
> trigger the bug).
>
> The complexity of the associated change in gcc makes it difficult to
> trace what actually changed to fix the underlying bug. If I had more
> time, I'd have dug in further and (tried to) come up with an answer.
>
> In reality, the (long long) cast could probably just be univerally
> applied without the conditional compilation. My only concern with that
> approach is that it risks turning the workaround into an arcane
> incantation whose meaning will eventually have been lost to time.
> ---
> include/linux/bitfield.h | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 54aeeef1f0ec7..12f5c5a3c8d72 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -46,6 +46,30 @@
>
> #define __bf_shf(x) (__builtin_ffsll(x) - 1)
>
> +#if defined(GCC_VERSION) && (GCC_VERSION < 140000)
> +/*
> + * This workaround is required for gcc<14. The issue is an interaction between
> + * FIELD_PREP_CONST() and __GENMASK_ULL() that can be boiled down this MCVE,
> + * which fails to compile as GCC doesn't recognize the expression as constant:
> + *
> + * int main() {
> + * sizeof(struct {
> + * int t : !(__builtin_ffsll(~0ULL) + 1 < 0);
> + * });
> + * }
> + *
> + * The underlying issue was inadvertently "fixed" (or perhaps sidestepped) in
> + * commit 0d00385eaf7 ("wide-int: Allow up to 16320 bits wide_int and change
> + * widest_int precision to 32640 bits [PR102989]"), which first appeared in
> + * GCC 14.1.
> + *
> + * See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124699
> + */
> +#define __const_bf_shf(x) __bf_shf((long long)(x))
> +#else
> +#define __const_bf_shf(x) __bf_shf(x)
> +#endif
> +
> #define __scalar_type_to_unsigned_cases(type) \
> unsigned type: (unsigned type)0, \
> signed type: (unsigned type)0
> @@ -157,11 +181,11 @@
> /* mask must be non-zero */ \
> BUILD_BUG_ON_ZERO((_mask) == 0) + \
> /* check if value fits */ \
> - BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
> + BUILD_BUG_ON_ZERO(~((_mask) >> __const_bf_shf(_mask)) & (_val)) + \
> /* check if mask is contiguous */ \
> - __BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) + \
> + __BF_CHECK_POW2((_mask) + (1ULL << __const_bf_shf(_mask))) + \
> /* and create the value */ \
> - (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)) \
> + (((typeof(_mask))(_val) << __const_bf_shf(_mask)) & (_mask)) \
> )
>
> /**
>
> ---
> base-commit: b20a9b5f9c4baeae0b2e143046b195b910c59714
> change-id: 20260324-field-prep-fix-fdfc4eb68156
>
>
prev parent reply other threads:[~2026-04-09 18:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-09 14:57 [PATCH] bitfield: Fix FIELD_PREP_CONST() with __GENMASK_ULL() on gcc<14 Matt Coster
2026-04-09 17:46 ` Yury Norov
2026-04-09 18:48 ` David Laight [this message]
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=20260409194833.3214139d@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=alessio.belle@imgtec.com \
--cc=alexandru.dadu@imgtec.com \
--cc=brajesh.gupta@imgtec.com \
--cc=frank.binns@imgtec.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=lkp@intel.com \
--cc=mailhol@kernel.org \
--cc=matt.coster@imgtec.com \
--cc=yury.norov@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox