min3() and max3() were added to optimise nested min(x, min(y, z))
sequences, bit only moved where the expansion was requiested.

Add a separate implementation for 3 argument calls.
These are never required to generate constant expressiions to
remove that logic.

Signed-off-by: David Laight <david.laight@aculab.com>
---
 include/linux/minmax.h | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2fb63efbeb0e..4bbc82c589cf 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -38,6 +38,11 @@
 	((__is_ok_signed(x) && __is_ok_signed(y)) ||	\
 	 (__is_ok_unsigned(x) && __is_ok_unsigned(y)))
 
+/* Check three values for min3(), max3() and clamp() */
+#define __types_ok3(x, y, z)							\
+	((__is_ok_signed(x) && __is_ok_signed(y) && __is_ok_signed(z)) ||	\
+	 (__is_ok_unsigned(x) && __is_ok_unsigned(y) && __is_ok_unsigned(z)))
+
 #define __cmp_op_min <
 #define __cmp_op_max >
 
@@ -90,13 +95,24 @@
  */
 #define umax(x, y)	__careful_cmp(max, __zero_extend(x), __zero_extend(y))
 
+#define __cmp_once3(op, x, y, z, uniq) ({	\
+	typeof(x) __x_##uniq = (x);		\
+	typeof(x) __y_##uniq = (y);		\
+	typeof(x) __z_##uniq = (z);		\
+	__cmp(op, __cmp(op, __x_##uniq, __y_##uniq), __z_##uniq); })
+
+#define __careful_cmp3(op, x, y, z, uniq) ({				\
+	static_assert(__types_ok3(x, y, z),				\
+		#op "3(" #x ", " #y ", " #z ") signedness error");	\
+	__cmp_once3(op, x, y, z, uniq); })
+
 /**
  * min3 - return minimum of three values
  * @x: first value
  * @y: second value
  * @z: third value
  */
-#define min3(x, y, z) min((typeof(x))min(x, y), z)
+#define min3(x, y, z) __careful_cmp3(min, x, y, z, __COUNTER__)
 
 /**
  * max3 - return maximum of three values
@@ -104,7 +120,7 @@
  * @y: second value
  * @z: third value
  */
-#define max3(x, y, z) max((typeof(x))max(x, y), z)
+#define max3(x, y, z) __careful_cmp3(max, x, y, z, __COUNTER__)
 
 /**
  * min_t - return minimum of two values, using the specified type
@@ -139,10 +155,9 @@
 	typeof(val) unique_val = (val);						\
 	typeof(lo) unique_lo = (lo);						\
 	typeof(hi) unique_hi = (hi);						\
-	_Static_assert(__if_constexpr((lo) <= (hi), (lo) <= (hi), true),		\
+	_Static_assert(__if_constexpr((lo) <= (hi), (lo) <= (hi), true),	\
 		"clamp() low limit " #lo " greater than high limit " #hi);	\
-	_Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");	\
-	_Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");	\
+	_Static_assert(__types_ok3(val, lo, hi), "clamp() signedness error");	\
 	__clamp(unique_val, unique_lo, unique_hi); })
 
 #define __careful_clamp(val, lo, hi) ({					\
-- 
2.17.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

On Wed, Jul 24, 2024, at 16:33, David Laight wrote:
> min3() and max3() were added to optimise nested min(x, min(y, z))
> sequences, bit only moved where the expansion was requiested.
>
> Add a separate implementation for 3 argument calls.
> These are never required to generate constant expressiions to
> remove that logic.
>
> Signed-off-by: David Laight <david.laight@aculab.com>

This brings another 3x improvement in the size of the expansion
and build speed.

> +#define __cmp_once3(op, x, y, z, uniq) ({	\
> +	typeof(x) __x_##uniq = (x);		\
> +	typeof(x) __y_##uniq = (y);		\
> +	typeof(x) __z_##uniq = (z);		\
> +	__cmp(op, __cmp(op, __x_##uniq, __y_##uniq), __z_##uniq); })

This still has a nested call to __cmp(), which makes the
resulting expression bigger than necessary.

The three typeof(x) should be x/y/z, right? Using __auto_type
would avoid the bug and also remove one more variable expansion.

Using another temporary variable, plus the use of __auto_type
brings the example line from xen/setup.c down 750KB to 530KB,
and the compile speed from 0.5s to 0.34s. 

 #define __cmp_once3(op, x, y, z, uniq) ({      \
       __auto_type __x_##uniq = (x);           \
       __auto_type __y_##uniq = (y);           \
       __auto_type __z_##uniq = (z);           \
       __auto_type __xy##uniq = __cmp(op, __x_##uniq, __y_##uniq); \
       __cmp(op, __xy_##uniq, __z_##uniq); })

The __auto_type change can also be applied to the other typeof()
in this file.

      Arnd

From: Arnd Bergmann
> Sent: 24 July 2024 18:04
> 
> On Wed, Jul 24, 2024, at 16:33, David Laight wrote:
> > min3() and max3() were added to optimise nested min(x, min(y, z))
> > sequences, bit only moved where the expansion was requiested.
> >
> > Add a separate implementation for 3 argument calls.
> > These are never required to generate constant expressiions to
> > remove that logic.
> >
> > Signed-off-by: David Laight <david.laight@aculab.com>
> 
> This brings another 3x improvement in the size of the expansion
> and build speed.
> 
> > +#define __cmp_once3(op, x, y, z, uniq) ({	\
> > +	typeof(x) __x_##uniq = (x);		\
> > +	typeof(x) __y_##uniq = (y);		\
> > +	typeof(x) __z_##uniq = (z);		\
> > +	__cmp(op, __cmp(op, __x_##uniq, __y_##uniq), __z_##uniq); })
> 
> This still has a nested call to __cmp(), which makes the
> resulting expression bigger than necessary.
> 
> The three typeof(x) should be x/y/z, right?

Ooops...

> Using __auto_type
> would avoid the bug and also remove one more variable expansion.

I'd thought that as well.
But hadn't looked up the syntax.

> Using another temporary variable, plus the use of __auto_type
> brings the example line from xen/setup.c down 750KB to 530KB,
> and the compile speed from 0.5s to 0.34s.
> 
>  #define __cmp_once3(op, x, y, z, uniq) ({      \
>        __auto_type __x_##uniq = (x);           \
>        __auto_type __y_##uniq = (y);           \
>        __auto_type __z_##uniq = (z);           \
>        __auto_type __xy##uniq = __cmp(op, __x_##uniq, __y_##uniq); \
>        __cmp(op, __xy_##uniq, __z_##uniq); })
> 
> The __auto_type change can also be applied to the other typeof()
> in this file.

True.

	David

> 
>       Arnd

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)