[PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

The kernel code style is to use !(expr) rather that (expr) == 0.
But clang complains that converting some constant expressions
(eg (0xffffu << 16)) to a boolean always evalutes to true.
This happens often in the validity checks in #defines.
Move tautological-constant-out-of-range-compare to W=2 (along with the
similar type-limits).

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 scripts/Makefile.warn | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.warn b/scripts/Makefile.warn
index 68e6fafcb80c..e2d467835c5b 100644
--- a/scripts/Makefile.warn
+++ b/scripts/Makefile.warn
@@ -151,7 +151,6 @@ KBUILD_CFLAGS += -Wformat-insufficient-args
 endif
 endif
 KBUILD_CFLAGS += $(call cc-option, -Wno-pointer-to-enum-cast)
-KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
 KBUILD_CFLAGS += $(call cc-option, -Wno-unaligned-access)
 KBUILD_CFLAGS += -Wno-enum-compare-conditional
 endif
@@ -179,6 +178,7 @@ KBUILD_CFLAGS += -Wno-shift-negative-value
 
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += -Wno-enum-enum-conversion
+KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
 endif
 
 ifdef CONFIG_CC_IS_GCC
-- 
2.39.5

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[Nathan Chancellor]

Hi David,

On Sun, Dec 14, 2025 at 01:15:28PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> The kernel code style is to use !(expr) rather that (expr) == 0.
> But clang complains that converting some constant expressions
> (eg (0xffffu << 16)) to a boolean always evalutes to true.
> This happens often in the validity checks in #defines.
> Move tautological-constant-out-of-range-compare to W=2 (along with the
> similar type-limits).
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>

I would like Arnd to comment on this before applying because the
reasoning of this change does not feel good enough to disable this
warning. It is not like '== 0' is inherently uncommon in the kernel or
hard to write to avoid the implicit conversion warning. To be honest, I
am a bit surprised -Wtautological-constant-out-of-range-compare fires
for that instead of some sort of -Wconversion warning...

> ---
>  scripts/Makefile.warn | 2 +-I
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.warn b/scripts/Makefile.warn
> index 68e6fafcb80c..e2d467835c5b 100644
> --- a/scripts/Makefile.warn
> +++ b/scripts/Makefile.warn
> @@ -151,7 +151,6 @@ KBUILD_CFLAGS += -Wformat-insufficient-args
>  endif
>  endif
>  KBUILD_CFLAGS += $(call cc-option, -Wno-pointer-to-enum-cast)
> -KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
>  KBUILD_CFLAGS += $(call cc-option, -Wno-unaligned-access)
>  KBUILD_CFLAGS += -Wno-enum-compare-conditional
>  endif
> @@ -179,6 +178,7 @@ KBUILD_CFLAGS += -Wno-shift-negative-value
>  
>  ifdef CONFIG_CC_IS_CLANG
>  KBUILD_CFLAGS += -Wno-enum-enum-conversion
> +KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
>  endif
>  
>  ifdef CONFIG_CC_IS_GCC
> -- 
> 2.39.5
> 

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[Arnd Bergmann]

On Fri, Dec 19, 2025, at 21:12, Nathan Chancellor wrote:
> On Sun, Dec 14, 2025 at 01:15:28PM +0000, david.laight.linux@gmail.com wrote:
>> From: David Laight <david.laight.linux@gmail.com>
>> 
>> The kernel code style is to use !(expr) rather that (expr) == 0.
>> But clang complains that converting some constant expressions
>> (eg (0xffffu << 16)) to a boolean always evalutes to true.
>> This happens often in the validity checks in #defines.
>> Move tautological-constant-out-of-range-compare to W=2 (along with the
>> similar type-limits).
>> 
>> Signed-off-by: David Laight <david.laight.linux@gmail.com>
>
> I would like Arnd to comment on this before applying because the
> reasoning of this change does not feel good enough to disable this
> warning. It is not like '== 0' is inherently uncommon in the kernel or
> hard to write to avoid the implicit conversion warning. To be honest, I
> am a bit surprised -Wtautological-constant-out-of-range-compare fires
> for that instead of some sort of -Wconversion warning...

I actually have the warning enabled by default in my randconfig
tree. I have a couple of fixup patches for the warnings we get
in the tree, but this should totally be fixable upstream as well.

See below for my patches. Are there any ones I missed?

    Arnd


From cb753c6f52ac5b8514c95be6263b2942ea71efd8 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 26 Jun 2025 19:59:06 +0200
Subject: [PATCH] riscv: limit sifive errate to CONFIG_64BIT

There are two errata for this vendor, and they both individually depend on
CONFIG_64BIT already. However, an 32-bit allmodconfig produces this warning
from clang for a condition that can never be true.

arch/riscv/errata/sifive/errata.c:29:14: error: result of comparison of constant 9223372036854775815 with expression of type 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
   29 |         if (arch_id != 0x8000000000000007 ||
      |             ~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~
arch/riscv/errata/sifive/errata.c:42:14: error: result of comparison of constant 9223372036854775815 with expression of type 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
   42 |         if (arch_id != 0x8000000000000007 && arch_id != 0x1)
      |             ~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index aca9b0cfcfec..1480abe54288 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -46,7 +46,7 @@ config ERRATA_MIPS_P8700_PAUSE_OPCODE
 
 config ERRATA_SIFIVE
 	bool "SiFive errata"
-	depends on RISCV_ALTERNATIVE
+	depends on RISCV_ALTERNATIVE && 64BIT
 	help
 	  All SiFive errata Kconfig depend on this Kconfig. Disabling
 	  this Kconfig will disable all SiFive errata. Please say "Y"
diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index d621b85dd63b..06ec2caaf73b 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -34,6 +34,7 @@ config ARCH_RENESAS
 config ARCH_SIFIVE
 	bool "SiFive SoCs"
 	select ERRATA_SIFIVE if !XIP_KERNEL
+	depends on 64BIT
 	help
 	  This enables support for SiFive SoC platform hardware.
 

From bcd1c82d604af688b6dc3b2a67454a47150b4d6d Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 20 Jun 2025 23:31:30 +0200
Subject: [PATCH] scsi: ibmvscsi_tgt: fix clang build warning

This is one of a handful of drivers that trigger the
-Wtautological-constant-out-of-range-compare warning flag:

drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1630:32: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]

Extend the type appropriately to avoid the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index f259746bc804..9ef6fefbf0ad 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1616,7 +1616,8 @@ static int ibmvscsis_cap_mad(struct scsi_info *vscsi, struct iu_entry *iue)
 	struct capabilities *cap;
 	struct mad_capability_common *common;
 	dma_addr_t token;
-	u16 olen, len, status, min_len, cap_len;
+	u16 len, status, min_len, cap_len;
+	u32 olen;
 	u32 flag;
 	uint flag_bits = 0;
 	long rc = 0;

From 523a92edf0b8e32a9b1da160d3edaebeb12cfccc Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 30 Jul 2024 16:25:07 +0200
Subject: [PATCH] fuse: fs-verity: aoid out-of-range comparison

clang points out that comparing the 16-bit size of the digest against
SIZE_MAX is not a helpful comparison:

fs/fuse/ioctl.c:130:18: error: result of comparison of constant 18446744073709551611 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
            ~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This either means tha tthe check can be removed entirely, or that the
intended comparison was for the 16-bit range. Assuming the latter was
intended, compare against U16_MAX instead.

Fixes: 9fe2a036a23c ("fuse: Add initial support for fs-verity")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index fdc175e93f74..c85b51ca7c86 100644
--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -129,7 +129,7 @@ static int fuse_setup_measure_verity(unsigned long arg, struct iovec *iov)
 	if (copy_from_user(&digest_size, &uarg->digest_size, sizeof(digest_size)))
 		return -EFAULT;
 
-	if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
+	if (digest_size > U16_MAX - sizeof(struct fsverity_digest))
 		return -EINVAL;
 
 	iov->iov_len = sizeof(struct fsverity_digest) + digest_size;

From bd340f4cb7fc43a8594f91de8b9305cc9f7a0f89 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 11 Mar 2021 16:52:40 +0100
Subject: [PATCH] [REVISIT 20210323] ARM: delay: avoid clang
 -Wtautological-constant warning

Passing an 8-bit constant into delay() triggers a warning when building
with 'make W=1' using clang:

drivers/clk/actions/owl-pll.c:182:2: error: result of comparison of constant 2000 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        udelay(pll_hw->delay);
        ^~~~~~~~~~~~~~~~~~~~~
arch/arm/include/asm/delay.h:84:9: note: expanded from macro 'udelay'
          ((n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() :              \
           ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~
arch/arm/mach-omap2/wd_timer.c:89:3: error: result of comparison of constant 2000 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
                udelay(oh->class->sysc->srst_udelay);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Shut up the warning by adding a cast to a 64-bit number. A cast to 'int'
would usually be sufficient, but would fail to cause a link-time error
for large 64-bit constants.

Link: https://lore.kernel.org/all/20210323133005.GC1463@shell.armlinux.org.uk/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index 1d069e558d8d..05a6a347d703 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -81,7 +81,7 @@ extern void __bad_udelay(void);
 
 #define udelay(n)							\
 	(__builtin_constant_p(n) ?					\
-	  ((n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() :		\
+	  ((u64)(n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() :		\
 			__const_udelay((n) * UDELAY_MULT)) :		\
 	  __udelay(n))
 

From 3080c47aa770504f1f9bb5fcd9c4c60147b6f48d Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 22 Sep 2021 13:11:34 +0200
Subject: [PATCH] [SUBMITTED 20240328] infiniband: uverbs: avoid out-of-range
 warnings

clang warns for comparisons that are always true, which is the case
for these two page size checks on architectures with 64KB pages:

drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE'
        int __ret_warn_on = !!(condition);                      \
                               ^~~~~~~~~
drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (hdr.length > PAGE_SIZE ||
            ~~~~~~~~~~ ^ ~~~~~~~~~

Add a cast to u32 in both cases, so it never warns about this.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index f80da6a67e24..e0cc3ddae71b 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -90,7 +90,7 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,
 		ALIGN(bundle_size + 256, sizeof(*pbundle->internal_buffer));
 
 	/* Do not want order-2 allocations for this. */
-	WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
+	WARN_ON_ONCE((u32)method_elm->bundle_size > PAGE_SIZE);
 }
 
 /**
@@ -636,7 +636,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	if (err)
 		return -EFAULT;
 
-	if (hdr.length > PAGE_SIZE ||
+	if ((u32)hdr.length > PAGE_SIZE ||
 	    hdr.length != struct_size(&hdr, attrs, hdr.num_attrs))
 		return -EINVAL;
 

From c47a0d27c29bcac2ea5ac9d8b0313258b19bcee4 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 22 Sep 2021 13:41:53 +0200
Subject: [PATCH] [SUBMITTED 20240328] ipv4: tcp_output: avoid warning about
 NET_ADD_STATS

Clang warns about a range check in percpu_add_op() being impossible
to hit for an u8 variable:

net/ipv4/tcp_output.c:188:3: error: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
                NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/ip.h:291:41: note: expanded from macro 'NET_ADD_STATS'
 #define NET_ADD_STATS(net, field, adnd) SNMP_ADD_STATS((net)->mib.net_statistics, field, adnd)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/snmp.h:143:4: note: expanded from macro 'SNMP_ADD_STATS'
                        this_cpu_add(mib->mibs[field], addend)
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/percpu-defs.h:509:33: note: expanded from macro 'this_cpu_add'
 #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
<scratch space>:187:1: note: expanded from here
this_cpu_add_8
^
arch/x86/include/asm/percpu.h:326:35: note: expanded from macro 'this_cpu_add_8'
 #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                              ((val) == 1 || (val) == -1)) ?            \
                                             ~~~~~ ^  ~~

Avoid this warning with a cast to a signed 'int'.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 479afb714bdf..00bf6c1639be 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -188,7 +188,7 @@ static inline void tcp_event_ack_sent(struct sock *sk, u32 rcv_nxt)
 
 	if (unlikely(tp->compressed_ack)) {
 		NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
-			      tp->compressed_ack);
+			      (int)tp->compressed_ack);
 		tp->compressed_ack = 0;
 		if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
 			__sock_put(sk);

From 8493af46865dd6a7f77dd7d8379181ac2b07e723 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 24 Sep 2021 12:50:07 +0200
Subject: [PATCH] [SUBMITTED 20240328] rbd: avoid out-of-range warning

clang-14 points out that the range check is always true on 64-bit
architectures since a u32 is not greater than the allowed size:

drivers/block/rbd.c:6079:17: error: result of comparison of constant 2305843009213693948 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
            ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is harmless, so just change the type of the temporary to size_t
to shut up that warning.

Fixes: bb23e37acb2a ("rbd: refactor rbd_header_from_disk()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index af0e21149dbc..1bb55827d18d 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -6054,7 +6054,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev,
 	void *p;
 	void *end;
 	u64 seq;
-	u32 snap_count;
+	size_t snap_count;
 	struct ceph_snap_context *snapc;
 	u32 i;
 

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[David Laight]

On Fri, 19 Dec 2025 13:12:31 -0700
Nathan Chancellor <nathan@kernel.org> wrote:

> Hi David,
> 
> On Sun, Dec 14, 2025 at 01:15:28PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > The kernel code style is to use !(expr) rather that (expr) == 0.
> > But clang complains that converting some constant expressions
> > (eg (0xffffu << 16)) to a boolean always evalutes to true.
> > This happens often in the validity checks in #defines.
> > Move tautological-constant-out-of-range-compare to W=2 (along with the
> > similar type-limits).
> > 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>  
> 
> I would like Arnd to comment on this before applying because the
> reasoning of this change does not feel good enough to disable this
> warning. It is not like '== 0' is inherently uncommon in the kernel or
> hard to write to avoid the implicit conversion warning. To be honest, I
> am a bit surprised -Wtautological-constant-out-of-range-compare fires
> for that instead of some sort of -Wconversion warning...

Somewhere I got confused and must have looked at the wrong email (or just
failed to separate two very long warning names).
The actual warning was:

>> drivers/gpu/drm/xe/xe_guc.c:639:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]  
     639 |                 klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);
         |                                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)

Inside FIELD_PREP_CONST(mask, val) there is (with the patch, and if I've
typed it correctly):
	BUILD_BUG_ON_ZERO(!(mask) || (mask) & ((mask) + ((mask) & -(mask)))))
to check the mask is non-zero and contiguous bits.

But this all reminds me of a compiler I once used that would generate a
warning for 'constant in conditional context'.

	David

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[Arnd Bergmann]

On Fri, Dec 19, 2025, at 23:18, David Laight wrote:
> On Fri, 19 Dec 2025 13:12:31 -0700 Nathan Chancellor <nathan@kernel.org> wrote:
>
> Somewhere I got confused and must have looked at the wrong email (or just
> failed to separate two very long warning names).
> The actual warning was:
>
>>> drivers/gpu/drm/xe/xe_guc.c:639:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]  
>      639 |                 klvs[count++] = 
> PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);

This does seem like a completely sensible warning to me, and it's
always been enabled by default. I see three patches in the git history
(all from Nathan), which all make sense as well.

> Inside FIELD_PREP_CONST(mask, val) there is (with the patch, and if I've
> typed it correctly):
> 	BUILD_BUG_ON_ZERO(!(mask) || (mask) & ((mask) + ((mask) & -(mask)))))
> to check the mask is non-zero and contiguous bits.

I think the problem is (as so often) the linux/bitfield.h headers
making things way too complicated. That condition makes no sense to
me, and neither would I expect a compiler to make sense of it either.

If there is no way to express those conditions more clearly, I would
prefer removing the BUILD_BUG_ON stuff from the bitfield.h header,
it keeps causing way more false positives than finding actual bugs
with the input.

     Arnd

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[David Laight]

On Sat, 20 Dec 2025 11:27:13 +0100
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Fri, Dec 19, 2025, at 23:18, David Laight wrote:
> > On Fri, 19 Dec 2025 13:12:31 -0700 Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Somewhere I got confused and must have looked at the wrong email (or just
> > failed to separate two very long warning names).
> > The actual warning was:
> >  
> >>> drivers/gpu/drm/xe/xe_guc.c:639:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]    
> >      639 |                 klvs[count++] = 
> > PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);  
> 
> This does seem like a completely sensible warning to me, and it's
> always been enabled by default. I see three patches in the git history
> (all from Nathan), which all make sense as well.
> 
> > Inside FIELD_PREP_CONST(mask, val) there is (with the patch, and if I've
> > typed it correctly):
> > 	BUILD_BUG_ON_ZERO(!(mask) || (mask) & ((mask) + ((mask) & -(mask)))))
> > to check the mask is non-zero and contiguous bits.  
> 
> I think the problem is (as so often) the linux/bitfield.h headers
> making things way too complicated. That condition makes no sense to
> me, and neither would I expect a compiler to make sense of it either.

It is simple really :-)
-mask is (~mask + 1) so its lowest set bit is the same at that of mask.
Adding mask changes the adjacent 1s to zeros.
Anding with mask is then any high bits that are the same in both.
So is non-zero if mask has noncontiguous bits in it.

Adding ' == 0' and ' != 0' would just make the line longer.

> 
> If there is no way to express those conditions more clearly, I would
> prefer removing the BUILD_BUG_ON stuff from the bitfield.h header,
> it keeps causing way more false positives than finding actual bugs
> with the input.

I was just trying to reduce the .i lines line from 18KB for a typical use.

But maybe the whole set of checks is entirely pointless.
The simple FIELD_PREP() is just ((val * (mask & -mask)) & mask).
FIELD_GET() can be (reg & mask)/(mask & -mask) for constants, but that
isn't 'nice' if mask is a variable, (reg & mask) >> ffs(mask) is better.
But you only want to use builtin_ffs() for constants so do need to
select between __ffs() and __ffs64() for variables - three cases.

There is also no point in the u8 and u16 variants (same for GENMASK()).
The values get promoted to 'int' and 'unsigned int' would be better.

Maybe I'll do a 'dump all the crap' commit.

Probably the only useful check is statically_true(hi < lo) in GENMASK.

	David

> 
>      Arnd

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[Arnd Bergmann]

On Sat, Dec 20, 2025, at 13:15, David Laight wrote:
> On Sat, 20 Dec 2025 11:27:13 +0100
>> 
>> This does seem like a completely sensible warning to me, and it's
>> always been enabled by default. I see three patches in the git history
>> (all from Nathan), which all make sense as well.
>> 
>> > Inside FIELD_PREP_CONST(mask, val) there is (with the patch, and if I've
>> > typed it correctly):
>> > 	BUILD_BUG_ON_ZERO(!(mask) || (mask) & ((mask) + ((mask) & -(mask)))))
>> > to check the mask is non-zero and contiguous bits.  
>> 
>> I think the problem is (as so often) the linux/bitfield.h headers
>> making things way too complicated. That condition makes no sense to
>> me, and neither would I expect a compiler to make sense of it either.
>
> It is simple really :-)
> -mask is (~mask + 1) so its lowest set bit is the same at that of mask.
> Adding mask changes the adjacent 1s to zeros.
> Anding with mask is then any high bits that are the same in both.
> So is non-zero if mask has noncontiguous bits in it.

The bit that I find most confusing here is how you have a boolean '||'
operation of two integers, but then interpret the result as an
integer again.

> Adding ' == 0' and ' != 0' would just make the line longer.

I don't think we care about the link length here at all.
Splitting it up into two BUILD_BUG_ON() or BUILD_BUG_ON_ZERO()
lines would help here as well.

>> If there is no way to express those conditions more clearly, I would
>> prefer removing the BUILD_BUG_ON stuff from the bitfield.h header,
>> it keeps causing way more false positives than finding actual bugs
>> with the input.
>
> I was just trying to reduce the .i lines line from 18KB for a typical use.

That seems like a worthwhile goal, but I think the only way to
make an actual impact here is to reduce the fan-out and evaluate
'mask' less than the current five times in that line (plus additional
evalations. If the 'mask' value is defined using complex macros
like ilog2() or max3() already, the expansion explodes.

Unfortunately the constant version of these macros can't use
compound statements, otherwise we could use an __auto_type temporary
here.

> Probably the only useful check is statically_true(hi < lo) in GENMASK.

Agreed, that one is clearly worth keeping.

    Arnd

Re: [PATCH 1/1] kbuild: Only enable -Wtautological-constant-out-of-range-compare for W=2

[David Laight]

On Mon, 22 Dec 2025 11:20:18 +0100
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Sat, Dec 20, 2025, at 13:15, David Laight wrote:
> > On Sat, 20 Dec 2025 11:27:13 +0100  
> >> 
> >> This does seem like a completely sensible warning to me, and it's
> >> always been enabled by default. I see three patches in the git history
> >> (all from Nathan), which all make sense as well.
> >>   
> >> > Inside FIELD_PREP_CONST(mask, val) there is (with the patch, and if I've
> >> > typed it correctly):
> >> > 	BUILD_BUG_ON_ZERO(!(mask) || (mask) & ((mask) + ((mask) & -(mask)))))
> >> > to check the mask is non-zero and contiguous bits.    
> >> 
> >> I think the problem is (as so often) the linux/bitfield.h headers
> >> making things way too complicated. That condition makes no sense to
> >> me, and neither would I expect a compiler to make sense of it either.  
> >
> > It is simple really :-)
> > -mask is (~mask + 1) so its lowest set bit is the same at that of mask.
> > Adding mask changes the adjacent 1s to zeros.
> > Anding with mask is then any high bits that are the same in both.
> > So is non-zero if mask has noncontiguous bits in it.  
> 
> The bit that I find most confusing here is how you have a boolean '||'
> operation of two integers, but then interpret the result as an
> integer again.

I'm not sure what you are getting at.
The BUILD_BUG() macros want a 'boolean' argument.
The _ON_ZERO() is the return value, nothing to do with the argument.
So LHS of the || is 'boolean' and the RHS has the implicit conversion.

> > Adding ' == 0' and ' != 0' would just make the line longer.  
> 
> I don't think we care about the link length here at all.
> Splitting it up into two BUILD_BUG_ON() or BUILD_BUG_ON_ZERO()
> lines would help here as well.

I'm merging them to reduce bloat.....
It's not as though either test fails often enough to need separate
error messages.

> >> If there is no way to express those conditions more clearly, I would
> >> prefer removing the BUILD_BUG_ON stuff from the bitfield.h header,
> >> it keeps causing way more false positives than finding actual bugs
> >> with the input.  
> >
> > I was just trying to reduce the .i lines line from 18KB for a typical use.  
> 
> That seems like a worthwhile goal, but I think the only way to
> make an actual impact here is to reduce the fan-out and evaluate
> 'mask' less than the current five times in that line (plus additional
> evalations. If the 'mask' value is defined using complex macros
> like ilog2() or max3() already, the expansion explodes.

It usually comes from GENMASK() and that is several hundred characters.
Changing type_max(type) to (type)-1 would help that a lot (type is unsigned).
Even changing type_max() to do (2 * (x - 1) + 1) instead of ((x - 1) + x)
saves quite a long expansion.

> Unfortunately the constant version of these macros can't use
> compound statements, otherwise we could use an __auto_type temporary
> here.

Fortunately there aren't that many uses of the _CONST version.
The change to bitmask.h that show this 'bug' used _auto_type
to reduce bloat for the common case.

> 
> > Probably the only useful check is statically_true(hi < lo) in GENMASK.  
> 
> Agreed, that one is clearly worth keeping.
> 
>     Arnd