RE: [PATCH next v2 03/11] minmax: Simplify signedness check

2024-02-27 Thread David Laight
From: kernel test robot
> Sent: 27 February 2024 01:34
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on drm-misc/drm-misc-next]
> [also build test WARNING on linux/master mkl-can-next/testing kdave/for-next 
> akpm-mm/mm-nonmm-unstable
> axboe-block/for-next linus/master v6.8-rc6 next-20240226]
> [cannot apply to next-20240223 dtor-input/next dtor-input/for-linus 
> horms-ipvs/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:
> https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-Put-all-the-clamp-
> definitions-together/20240226-005902
> base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> patch link:
> https://lore.kernel.org/r/8657dd5c2264456f8a005520a3b90e2b%40AcuMS.aculab.com
> patch subject: [PATCH next v2 03/11] minmax: Simplify signedness check
> config: alpha-defconfig 
> (https://download.01.org/0day-ci/archive/20240227/202402270937.9kmO5PFt-
> l...@intel.com/config)
> compiler: alpha-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-
> ci/archive/20240227/202402270937.9kmo5pft-...@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version 
> of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot 
> | Closes: 
> https://lore.kernel.org/oe-kbuild-all/202402270937.9kmo5pft-...@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>In file included from include/linux/kernel.h:28,
> from include/linux/cpumask.h:10,
> from include/linux/smp.h:13,
> from include/linux/lockdep.h:14,
> from include/linux/spinlock.h:63,
> from include/linux/swait.h:7,
> from include/linux/completion.h:12,
> from include/linux/crypto.h:15,
> from include/crypto/aead.h:13,
> from include/crypto/internal/aead.h:11,
> from crypto/skcipher.c:12:
>crypto/skcipher.c: In function 'skcipher_get_spot':
> >> include/linux/minmax.h:31:70: warning: ordered comparison of pointer with 
> >> integer zero [-Wextra]
>   31 | (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + 
> 0 >= 0 : 0))

Hmmm -Wextra isn't normally set.
But I do wish the compiler would do dead code elimination before
these warnings.

Apart from stopping code using min()/max() for pointer types
(all the type checking is pointless) I think that __is_constextr()
can be implemented using _Generic (instead of sizeof(type)) and then the
true/false return values can be specified and need not be the same types.
That test can then be:
(__if_constexpr(x, x, -1) >= 0)
(The '+ 0' is there to convert bool to int and won't be needed
for non-constant bool.)

I may drop the last few patches until MIN/MAX have been removed
from everywhere else to free up the names.

David

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



Re: [PATCH next v2 03/11] minmax: Simplify signedness check

2024-02-26 Thread kernel test robot
Hi David,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linux/master mkl-can-next/testing kdave/for-next 
akpm-mm/mm-nonmm-unstable axboe-block/for-next linus/master v6.8-rc6 
next-20240226]
[cannot apply to next-20240223 dtor-input/next dtor-input/for-linus 
horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-Put-all-the-clamp-definitions-together/20240226-005902
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/8657dd5c2264456f8a005520a3b90e2b%40AcuMS.aculab.com
patch subject: [PATCH next v2 03/11] minmax: Simplify signedness check
config: alpha-defconfig 
(https://download.01.org/0day-ci/archive/20240227/202402270937.9kmo5pft-...@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240227/202402270937.9kmo5pft-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202402270937.9kmo5pft-...@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:28,
from include/linux/cpumask.h:10,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/spinlock.h:63,
from include/linux/swait.h:7,
from include/linux/completion.h:12,
from include/linux/crypto.h:15,
from include/crypto/aead.h:13,
from include/crypto/internal/aead.h:11,
from crypto/skcipher.c:12:
   crypto/skcipher.c: In function 'skcipher_get_spot':
>> include/linux/minmax.h:31:70: warning: ordered comparison of pointer with 
>> integer zero [-Wextra]
  31 | (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + 0 
>= 0 : 0))
 |  
^~
   include/linux/minmax.h:39:11: note: in expansion of macro '__is_ok_unsigned'
  39 |  (__is_ok_unsigned(x) && __is_ok_unsigned(y)))
 |   ^~~~
   include/linux/minmax.h:49:24: note: in expansion of macro '__types_ok'
  49 | _Static_assert(__types_ok(x, y),\
 |^~
   include/linux/minmax.h:56:17: note: in expansion of macro '__cmp_once'
  56 | __cmp_once(op, x, y, __UNIQUE_ID(__x), 
__UNIQUE_ID(__y)))
 | ^~
   include/linux/minmax.h:70:25: note: in expansion of macro '__careful_cmp'
  70 | #define max(x, y)   __careful_cmp(max, x, y)
 | ^
   crypto/skcipher.c:83:16: note: in expansion of macro 'max'
  83 | return max(start, end_page);
 |^~~
>> include/linux/minmax.h:31:70: warning: ordered comparison of pointer with 
>> integer zero [-Wextra]
  31 | (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + 0 
>= 0 : 0))
 |  
^~
   include/linux/minmax.h:39:34: note: in expansion of macro '__is_ok_unsigned'
  39 |  (__is_ok_unsigned(x) && __is_ok_unsigned(y)))
 |  ^~~~
   include/linux/minmax.h:49:24: note: in expansion of macro '__types_ok'
  49 | _Static_assert(__types_ok(x, y),\
 |^~
   include/linux/minmax.h:56:17: note: in expansion of macro '__cmp_once'
  56 | __cmp_once(op, x, y, __UNIQUE_ID(__x), 
__UNIQUE_ID(__y)))
 | ^~
   include/linux/minmax.h:70:25: note: in expansion of macro '__careful_cmp'
  70 | #define max(x, y)   __careful_cmp(max, x, y)
 | ^
   crypto/skcipher.c:83:16: note: in expansion of macro 'max'
  83 | return max(start, end_page);
 |^~~


vim +31 include/linux/minmax.h

 9  
10  /*
11   * min()/max()/clamp() macros must accomplish several things:
12   *
13   * - Avoid multiple evaluations of the arguments (so side-effects like
14   *   "x++" happen only once) when non-constant.
15   * - Retain result as a constant expressions when called with only
16   *   constant expressions (to avoid tripping VLA warnings in stack
17   *   allocation usage).