diff mbox series

[05/10] minmax: simplify __clamp_once() by using is_const_false()

Message ID 20241203-is_constexpr-refactor-v1-5-4e4cbaecc216@wanadoo.fr (mailing list archive)
State New
Headers show
Series compiler.h: refactor __is_constexpr() into is_const{,_true,_false}() | expand

Commit Message

Vincent Mailhol via B4 Relay Dec. 2, 2024, 5:33 p.m. UTC
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

In __clamp_once(),

  __builtin_choose_expr(__is_constexpr((lo) > (hi)), (lo) <= (hi), true)

is equivalent to:

  !is_const_false((lo) <= (hi))

Apply is_const_false() to simplify __clamp_once().

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 include/linux/minmax.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

David Laight Dec. 4, 2024, 6:54 p.m. UTC | #1
From: Vincent Mailhol
> Sent: 02 December 2024 17:33
> 
> In __clamp_once(),
> 
>   __builtin_choose_expr(__is_constexpr((lo) > (hi)), (lo) <= (hi), true)
> 
> is equivalent to:
> 
>   !is_const_false((lo) <= (hi))
> 
> Apply is_const_false() to simplify __clamp_once().

There is already a patch 'for next' that changes it use BUILD_BUG_ON_MSG()
and statically_true().

It has found some 'interesting' code.

	David

> 
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
>  include/linux/minmax.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/linux/minmax.h b/include/linux/minmax.h
> index 98008dd92153db10c672155bca93201ffabee994..431bf76ac460a11a2e4af23acd90c0d26e99c862 100644
> --- a/include/linux/minmax.h
> +++ b/include/linux/minmax.h
> @@ -111,8 +111,7 @@
>  	__auto_type uval = (val);						\
>  	__auto_type ulo = (lo);							\
>  	__auto_type uhi = (hi);							\
> -	static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), 	\
> -			(lo) <= (hi), true),					\
> +	static_assert(!is_const_false((lo) <= (hi)),				\
>  		"clamp() low limit " #lo " greater than high limit " #hi);	\
>  	BUILD_BUG_ON_MSG(!__types_ok3(val,lo,hi,uval,ulo,uhi),			\
>  		"clamp("#val", "#lo", "#hi") signedness error");		\
> 
> --
> 2.45.2
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Vincent Mailhol Dec. 5, 2024, 3:52 p.m. UTC | #2
On Thu. 5 Dec. 2024 at 03:54, David Laight <David.Laight@aculab.com> wrote:
> From: Vincent Mailhol
> > Sent: 02 December 2024 17:33
> >
> > In __clamp_once(),
> >
> >   __builtin_choose_expr(__is_constexpr((lo) > (hi)), (lo) <= (hi), true)
> >
> > is equivalent to:
> >
> >   !is_const_false((lo) <= (hi))
> >
> > Apply is_const_false() to simplify __clamp_once().
>
> There is already a patch 'for next' that changes it use BUILD_BUG_ON_MSG()
> and statically_true().

Found it!

  https://lore.kernel.org/all/34d53778977747f19cce2abb287bb3e6@AcuMS.aculab.com/

I think the easiest would be for me to cherry pick this patch. So that
regardless which series is merged first, no conflict will occur, the
patch will just be skipped the second time it is encountered.

Does this work for you?


Yours sincerely,
Vincent Mailhol
Vincent Mailhol Dec. 9, 2024, 12:32 p.m. UTC | #3
On Fri. 6 Dec. 2024 at 00:52, Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
> On Thu. 5 Dec. 2024 at 03:54, David Laight <David.Laight@aculab.com> wrote:
> > From: Vincent Mailhol
> > > Sent: 02 December 2024 17:33
> > >
> > > In __clamp_once(),
> > >
> > >   __builtin_choose_expr(__is_constexpr((lo) > (hi)), (lo) <= (hi), true)
> > >
> > > is equivalent to:
> > >
> > >   !is_const_false((lo) <= (hi))
> > >
> > > Apply is_const_false() to simplify __clamp_once().
> >
> > There is already a patch 'for next' that changes it use BUILD_BUG_ON_MSG()
> > and statically_true().
>
> Found it!
>
>   https://lore.kernel.org/all/34d53778977747f19cce2abb287bb3e6@AcuMS.aculab.com/

I picked up your patch and got two build errors on an allyesconfig.

You already sent a patch for the first one:

  https://lore.kernel.org/all/33893212b1cc4a418cec09aeeed0a9fc@AcuMS.aculab.com/

For the second one, I submitted a patch here:

  https://lore.kernel.org/all/20241209-nfs4state_fix-v1-1-7a66819c60f0@wanadoo.fr/

I will wait for those two to appear in Andrew's mm tree first, and
only then, I will send a v2 (that will be rebased on the mm tree to
get your change).

Meanwhile, I think this series will be on hiatus.


Yours sincerely,
Vincent Mailhol
diff mbox series

Patch

diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 98008dd92153db10c672155bca93201ffabee994..431bf76ac460a11a2e4af23acd90c0d26e99c862 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -111,8 +111,7 @@ 
 	__auto_type uval = (val);						\
 	__auto_type ulo = (lo);							\
 	__auto_type uhi = (hi);							\
-	static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), 	\
-			(lo) <= (hi), true),					\
+	static_assert(!is_const_false((lo) <= (hi)),				\
 		"clamp() low limit " #lo " greater than high limit " #hi);	\
 	BUILD_BUG_ON_MSG(!__types_ok3(val,lo,hi,uval,ulo,uhi),			\
 		"clamp("#val", "#lo", "#hi") signedness error");		\