RE: [PATCH] media: solo6x10: replace max(a, min(b, c)) by clamp(b, a, c)

[David Laight]

From: Aurelien Jarno
> Sent: 13 January 2024 18:34
> 
> This patch replaces max(a, min(b, c)) by clamp(b, a, c) in the solo6x10
> driver. This improves the readability and more importantly, for the
> solo6x10-p2m.c file, this reduces on my system (x86-64, gcc 13):
> - the preprocessed size from 121 MiB to 4.5 MiB;
> - the build CPU time from 46.8 s to 1.6 s;
> - the build memory from 2786 MiB to 98MiB.
> 
> In fine, this allows this relatively simple C file to be built on a
> 32-bit system.
> 
> Reported-by: Jiri Slaby <jirislaby@xxxxxxxxx>
> Closes: https://lore.kernel.org/lkml/18c6df0d-45ed-450c-9eda-95160a2bbb8e@xxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx # v6.7+
> Suggested-by: David Laight <David.Laight@xxxxxxxxxx>
> Signed-off-by: Aurelien Jarno <aurelien@xxxxxxxxxxx>

Reviewed-by: David Laight <David.Laight@xxxxxxxxxx>

I was about to send the same patch.
Although I'm not sure why the cpu time is so large.
It compiles pretty immediately on my system.
I do have some patches to minmax.h that reduce the .i for the nested
clamp() to around 200k.
Mostly obtained be adding min/max_const() for the few places
that need a constant and min/max_ptr() for pointer types.
Supporting both causes the expansion to be a lot larger.

	David

> ---
>  drivers/media/pci/solo6x10/solo6x10-offsets.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-offsets.h b/drivers/media/pci/solo6x10/solo6x10-
> offsets.h
> index f414ee1316f2..fdbb817e6360 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-offsets.h
> +++ b/drivers/media/pci/solo6x10/solo6x10-offsets.h
> @@ -57,16 +57,16 @@
>  #define SOLO_MP4E_EXT_ADDR(__solo) \
>  	(SOLO_EREF_EXT_ADDR(__solo) + SOLO_EREF_EXT_AREA(__solo))
>  #define SOLO_MP4E_EXT_SIZE(__solo) \
> -	max((__solo->nr_chans * 0x00080000),				\
> -	    min(((__solo->sdram_size - SOLO_MP4E_EXT_ADDR(__solo)) -	\
> -		 __SOLO_JPEG_MIN_SIZE(__solo)), 0x00ff0000))
> +	clamp(__solo->sdram_size - SOLO_MP4E_EXT_ADDR(__solo) -	\
> +	      __SOLO_JPEG_MIN_SIZE(__solo),			\
> +	      __solo->nr_chans * 0x00080000, 0x00ff0000)
> 
>  #define __SOLO_JPEG_MIN_SIZE(__solo)		(__solo->nr_chans * 0x00080000)
>  #define SOLO_JPEG_EXT_ADDR(__solo) \
>  		(SOLO_MP4E_EXT_ADDR(__solo) + SOLO_MP4E_EXT_SIZE(__solo))
>  #define SOLO_JPEG_EXT_SIZE(__solo) \
> -	max(__SOLO_JPEG_MIN_SIZE(__solo),				\
> -	    min((__solo->sdram_size - SOLO_JPEG_EXT_ADDR(__solo)), 0x00ff0000))
> +	clamp(__solo->sdram_size - SOLO_JPEG_EXT_ADDR(__solo),	\
> +	      __SOLO_JPEG_MIN_SIZE(__solo), 0x00ff0000)
> 
>  #define SOLO_SDRAM_END(__solo) \
>  	(SOLO_JPEG_EXT_ADDR(__solo) + SOLO_JPEG_EXT_SIZE(__solo))
> --
> 2.42.0

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