[PATCH v2] arc: Fix __fls() const-foldability via __builtin_clzl()

[PATCH v2] arc: Fix __fls() const-foldability via __builtin_clzl()

[Kees Cook]

While tracking down a problem where constant expressions used by
BUILD_BUG_ON() suddenly stopped working[1], we found that an added static
initializer was convincing the compiler that it couldn't track the state
of the prior statically initialized value. Tracing this down found that
ffs() was used in the initializer macro, but since it wasn't marked with
__attribute__const__, the compiler had to assume the function might
change variable states as a side-effect (which is not true for ffs(),
which provides deterministic math results).

For arc architecture with CONFIG_ISA_ARCV2=y, the __fls() function
uses __builtin_arc_fls() which lacks GCC's const attribute, preventing
compile-time constant folding, and KUnit testing of ffs/fls fails on
arc[3]. A patch[2] to GCC to solve this has been sent.

Add a fix for this by handling compile-time constants with the standard
__builtin_clzl() builtin (which has const attribute) while preserving
the optimized arc-specific builtin for runtime cases. This has the added
benefit of skipping runtime calculation of compile-time constant values.
Even with the GCC bug fixed (which is about "attribute const") this is a
good change to avoid needless runtime costs, and should be done
regardless of the state of GCC's bug.

Build tested ARCH=arc allyesconfig with GCC arc-linux 15.2.0.

Link: https://github.com/KSPP/linux/issues/364 [1]
Link: https://gcc.gnu.org/pipermail/gcc-patches/2025-August/693273.html
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202508031025.doWxtzzc-lkp@intel.com/ [3]
Signed-off-by: Kees Cook <kees@kernel.org>
---
 v2: clarify commit log (we want this patch regardless of GCC being fixed)
 v1: https://lore.kernel.org/lkml/20250826034354.work.684-kees@kernel.org/
---
 arch/arc/include/asm/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arc/include/asm/bitops.h b/arch/arc/include/asm/bitops.h
index 5340c2871392..df894235fdbc 100644
--- a/arch/arc/include/asm/bitops.h
+++ b/arch/arc/include/asm/bitops.h
@@ -133,6 +133,8 @@ static inline __attribute__ ((const)) int fls(unsigned int x)
  */
 static inline __attribute__ ((const)) unsigned long __fls(unsigned long x)
 {
+	if (__builtin_constant_p(x))
+		return x ? BITS_PER_LONG - 1 - __builtin_clzl(x) : 0;
 	/* FLS insn has exactly same semantics as the API */
 	return	__builtin_arc_fls(x);
 }
-- 
2.34.1

Re: [PATCH v2] arc: Fix __fls() const-foldability via __builtin_clzl()

[Vineet Gupta]

On 8/30/25 19:23, Kees Cook wrote:
> While tracking down a problem where constant expressions used by
> BUILD_BUG_ON() suddenly stopped working[1], we found that an added static
> initializer was convincing the compiler that it couldn't track the state
> of the prior statically initialized value. Tracing this down found that
> ffs() was used in the initializer macro, but since it wasn't marked with
> __attribute__const__, the compiler had to assume the function might
> change variable states as a side-effect (which is not true for ffs(),
> which provides deterministic math results).
>
> For arc architecture with CONFIG_ISA_ARCV2=y, the __fls() function
> uses __builtin_arc_fls() which lacks GCC's const attribute, preventing
> compile-time constant folding, and KUnit testing of ffs/fls fails on
> arc[3]. A patch[2] to GCC to solve this has been sent.
>
> Add a fix for this by handling compile-time constants with the standard
> __builtin_clzl() builtin (which has const attribute) while preserving
> the optimized arc-specific builtin for runtime cases. This has the added
> benefit of skipping runtime calculation of compile-time constant values.
> Even with the GCC bug fixed (which is about "attribute const") this is a
> good change to avoid needless runtime costs, and should be done
> regardless of the state of GCC's bug.
>
> Build tested ARCH=arc allyesconfig with GCC arc-linux 15.2.0.
>
> Link: https://github.com/KSPP/linux/issues/364 [1]
> Link: https://gcc.gnu.org/pipermail/gcc-patches/2025-August/693273.html
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202508031025.doWxtzzc-lkp@intel.com/ [3]
> Signed-off-by: Kees Cook <kees@kernel.org>

Acked-by: Vineet Gupta <vgupta@kernel.org>

Phew ! Thanks for taking time to track this down and also submitting the gcc
fix, like a true open source Good Samaritan.
Please let me know if you want me to take it via ARC tree, just be advised it
will be delayed due to slow'ish development.

Cheers,
-Vineet


> ---
>  v2: clarify commit log (we want this patch regardless of GCC being fixed)
>  v1: https://lore.kernel.org/lkml/20250826034354.work.684-kees@kernel.org/
> ---
>  arch/arc/include/asm/bitops.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arc/include/asm/bitops.h b/arch/arc/include/asm/bitops.h
> index 5340c2871392..df894235fdbc 100644
> --- a/arch/arc/include/asm/bitops.h
> +++ b/arch/arc/include/asm/bitops.h
> @@ -133,6 +133,8 @@ static inline __attribute__ ((const)) int fls(unsigned int x)
>   */
>  static inline __attribute__ ((const)) unsigned long __fls(unsigned long x)
>  {
> +	if (__builtin_constant_p(x))
> +		return x ? BITS_PER_LONG - 1 - __builtin_clzl(x) : 0;
>  	/* FLS insn has exactly same semantics as the API */
>  	return	__builtin_arc_fls(x);
>  }

Re: [PATCH v2] arc: Fix __fls() const-foldability via __builtin_clzl()

[Yury Norov]

On Sat, Aug 30, 2025 at 07:23:53PM -0700, Kees Cook wrote:
> While tracking down a problem where constant expressions used by
> BUILD_BUG_ON() suddenly stopped working[1], we found that an added static
> initializer was convincing the compiler that it couldn't track the state
> of the prior statically initialized value. Tracing this down found that
> ffs() was used in the initializer macro, but since it wasn't marked with
> __attribute__const__, the compiler had to assume the function might
> change variable states as a side-effect (which is not true for ffs(),
> which provides deterministic math results).
> 
> For arc architecture with CONFIG_ISA_ARCV2=y, the __fls() function
> uses __builtin_arc_fls() which lacks GCC's const attribute, preventing
> compile-time constant folding, and KUnit testing of ffs/fls fails on
> arc[3]. A patch[2] to GCC to solve this has been sent.
> 
> Add a fix for this by handling compile-time constants with the standard
> __builtin_clzl() builtin (which has const attribute) while preserving
> the optimized arc-specific builtin for runtime cases. This has the added
> benefit of skipping runtime calculation of compile-time constant values.
> Even with the GCC bug fixed (which is about "attribute const") this is a
> good change to avoid needless runtime costs, and should be done
> regardless of the state of GCC's bug.
> 
> Build tested ARCH=arc allyesconfig with GCC arc-linux 15.2.0.
> 
> Link: https://github.com/KSPP/linux/issues/364 [1]
> Link: https://gcc.gnu.org/pipermail/gcc-patches/2025-August/693273.html
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202508031025.doWxtzzc-lkp@intel.com/ [3]
> Signed-off-by: Kees Cook <kees@kernel.org>

Applied in bitmap-for-next, thanks!