* drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y
@ 2025-01-29 14:02 kernel test robot
2025-01-29 22:21 ` David Laight
0 siblings, 1 reply; 7+ messages in thread
From: kernel test robot @ 2025-01-29 14:02 UTC (permalink / raw)
To: Even Xu
Cc: oe-kbuild-all, linux-kernel, Jiri Kosina, Xinpeng Sun,
Srinivas Pandruvada, Mark Pearson
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5
commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc-hid: intel-thc: Add THC DMA interfaces
date: 3 weeks ago
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ovr-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ovr-lkp@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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y
drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h, ...):
include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
287
288 static void release_dma_buffers(struct thc_device *dev,
289 struct thc_dma_configuration *config)
290 {
291 size_t prd_tbls_size = array_size(PRD_TABLE_SIZE, config->prd_tbl_num);
292 unsigned int i;
293
294 if (!config->is_enabled)
295 return;
296
297 for (i = 0; i < config->prd_tbl_num; i++) {
> 298 if (!config->sgls[i] | !config->sgls_nent[i])
299 continue;
300
301 dma_unmap_sg(dev->dev, config->sgls[i],
302 config->sgls_nent[i],
303 config->dir);
304
305 sgl_free(config->sgls[i]);
306 config->sgls[i] = NULL;
307 }
308
309 memset(config->prd_tbls, 0, prd_tbls_size);
310
311 if (config->prd_tbls) {
312 dma_free_coherent(dev->dev, prd_tbls_size, config->prd_tbls,
313 config->prd_tbls_dma_handle);
314 config->prd_tbls = NULL;
315 config->prd_tbls_dma_handle = 0;
316 }
317 }
318
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y 2025-01-29 14:02 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y kernel test robot @ 2025-01-29 22:21 ` David Laight 2025-02-05 2:56 ` Xu, Even 0 siblings, 1 reply; 7+ messages in thread From: David Laight @ 2025-01-29 22:21 UTC (permalink / raw) To: kernel test robot Cc: Even Xu, oe-kbuild-all, linux-kernel, Jiri Kosina, Xinpeng Sun, Srinivas Pandruvada, Mark Pearson On Wed, 29 Jan 2025 22:02:59 +0800 kernel test robot <lkp@intel.com> wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5 > commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc-hid: intel-thc: Add THC DMA interfaces > date: 3 weeks ago > config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ovr-lkp@intel.com/config) > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ovr-lkp@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 <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr-lkp@intel.com/ > > sparse warnings: (new ones prefixed by >>) > >> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h, ...): > include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false > include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false > > vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c ... > 297 for (i = 0; i < config->prd_tbl_num; i++) { > > 298 if (!config->sgls[i] | !config->sgls_nent[i]) > 299 continue; ... If zeros are unlikely the bit-wise 'or' is pretty likely to generate better code than a logical 'or'. David ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y 2025-01-29 22:21 ` David Laight @ 2025-02-05 2:56 ` Xu, Even 2025-02-05 21:56 ` David Laight 0 siblings, 1 reply; 7+ messages in thread From: Xu, Even @ 2025-02-05 2:56 UTC (permalink / raw) To: David Laight, lkp Cc: oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, Jiri Kosina, Sun, Xinpeng, Srinivas Pandruvada, Mark Pearson > -----Original Message----- > From: David Laight <david.laight.linux@gmail.com> > Sent: Thursday, January 30, 2025 6:22 AM > To: lkp <lkp@intel.com> > Cc: Xu, Even <even.xu@intel.com>; oe-kbuild-all@lists.linux.dev; linux- > kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, Xinpeng > <xinpeng.sun@intel.com>; Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > lenovo@squebb.ca> > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > sparse: dubious: !x | !y > > On Wed, 29 Jan 2025 22:02:59 +0800 > kernel test robot <lkp@intel.com> wrote: > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5 > > commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc-hid: > intel-thc: Add THC DMA interfaces > > date: 3 weeks ago > > config: x86_64-allyesconfig > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ov > > r-lkp@intel.com/config) > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 > > build): > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ov > > r-lkp@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 <lkp@intel.com> > > | Closes: > > | https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr-lkp@inte > > | l.com/ > > > > sparse warnings: (new ones prefixed by >>) > > >> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > >> sparse: dubious: !x | !y > > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: in included file > (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h, ...): > > include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always > evaluates to false > > include/linux/page-flags.h:237:46: sparse: sparse: self-comparison > > always evaluates to false > > > > vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c > ... > > 297 for (i = 0; i < config->prd_tbl_num; i++) { > > > 298 if (!config->sgls[i] | !config->sgls_nent[i]) > > 299 continue; > ... > > If zeros are unlikely the bit-wise 'or' is pretty likely to generate better code than a > logical 'or'. > > David Good suggestion! So the code can be optimized to: for (i = 0; i < config->prd_tbl_num; i++) { if (config->sgls[i] && config->sgls_nent[i]) { ...... } } Will create a patch for this. Best Regards, Even Xu ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y 2025-02-05 2:56 ` Xu, Even @ 2025-02-05 21:56 ` David Laight 2025-02-06 2:50 ` Xu, Even 0 siblings, 1 reply; 7+ messages in thread From: David Laight @ 2025-02-05 21:56 UTC (permalink / raw) To: Xu, Even Cc: lkp, oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, Jiri Kosina, Sun, Xinpeng, Srinivas Pandruvada, Mark Pearson On Wed, 5 Feb 2025 02:56:05 +0000 "Xu, Even" <even.xu@intel.com> wrote: > > -----Original Message----- > > From: David Laight <david.laight.linux@gmail.com> > > Sent: Thursday, January 30, 2025 6:22 AM > > To: lkp <lkp@intel.com> > > Cc: Xu, Even <even.xu@intel.com>; oe-kbuild-all@lists.linux.dev; linux- > > kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, Xinpeng > > <xinpeng.sun@intel.com>; Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > > lenovo@squebb.ca> > > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > sparse: dubious: !x | !y > > > > On Wed, 29 Jan 2025 22:02:59 +0800 > > kernel test robot <lkp@intel.com> wrote: > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > > head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5 > > > commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc-hid: > > intel-thc: Add THC DMA interfaces > > > date: 3 weeks ago > > > config: x86_64-allyesconfig > > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ov > > > r-lkp@intel.com/config) > > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 > > > build): > > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFDq4ov > > > r-lkp@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 <lkp@intel.com> > > > | Closes: > > > | https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr-lkp@inte > > > | l.com/ > > > > > > sparse warnings: (new ones prefixed by >>) > > > >> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > > >> sparse: dubious: !x | !y > > > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: in included file > > (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h, ...): > > > include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always > > evaluates to false > > > include/linux/page-flags.h:237:46: sparse: sparse: self-comparison > > > always evaluates to false > > > > > > vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c > > ... > > > 297 for (i = 0; i < config->prd_tbl_num; i++) { > > > > 298 if (!config->sgls[i] | !config->sgls_nent[i]) > > > 299 continue; > > ... > > > > If zeros are unlikely the bit-wise 'or' is pretty likely to generate better code than a > > logical 'or'. > > > > David > > Good suggestion! > So the code can be optimized to: > for (i = 0; i < config->prd_tbl_num; i++) { > if (config->sgls[i] && config->sgls_nent[i]) { > ...... > } That just adds a level of indentation to the source. It'll generate much the same code as !x || !y. Both will generate two conditional branches. It almost certainly makes in immeasurable difference here, but !x | !y can be generated using (on x86) the 'sete' instruction to get a 0/1 from the x == 0 compare (that sets the flags), and then doing an 'or' and a single jump. David > } > > Will create a patch for this. > > Best Regards, > Even Xu > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y 2025-02-05 21:56 ` David Laight @ 2025-02-06 2:50 ` Xu, Even 2025-02-06 9:04 ` David Laight 0 siblings, 1 reply; 7+ messages in thread From: Xu, Even @ 2025-02-06 2:50 UTC (permalink / raw) To: David Laight Cc: lkp, oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, Jiri Kosina, Sun, Xinpeng, Srinivas Pandruvada, Mark Pearson > -----Original Message----- > From: David Laight <david.laight.linux@gmail.com> > Sent: Thursday, February 6, 2025 5:57 AM > To: Xu, Even <even.xu@intel.com> > Cc: lkp <lkp@intel.com>; oe-kbuild-all@lists.linux.dev; linux- > kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, Xinpeng > <xinpeng.sun@intel.com>; Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > lenovo@squebb.ca> > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > sparse: dubious: !x | !y > > On Wed, 5 Feb 2025 02:56:05 +0000 > "Xu, Even" <even.xu@intel.com> wrote: > > > > -----Original Message----- > > > From: David Laight <david.laight.linux@gmail.com> > > > Sent: Thursday, January 30, 2025 6:22 AM > > > To: lkp <lkp@intel.com> > > > Cc: Xu, Even <even.xu@intel.com>; oe-kbuild-all@lists.linux.dev; > > > linux- kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, > > > Xinpeng <xinpeng.sun@intel.com>; Srinivas Pandruvada > > > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > > > lenovo@squebb.ca> > > > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: > sparse: > > > sparse: dubious: !x | !y > > > > > > On Wed, 29 Jan 2025 22:02:59 +0800 > > > kernel test robot <lkp@intel.com> wrote: > > > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > > > > head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5 > > > > commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc-hid: > > > intel-thc: Add THC DMA interfaces > > > > date: 3 weeks ago > > > > config: x86_64-allyesconfig > > > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFD > > > > q4ov > > > > r-lkp@intel.com/config) > > > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a > > > > W=1 > > > > build): > > > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFD > > > > q4ov > > > > r-lkp@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 <lkp@intel.com> > > > > | Closes: > > > > | https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr-lkp@ > > > > | inte > > > > | l.com/ > > > > > > > > sparse warnings: (new ones prefixed by >>) > > > > >> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > > > >> sparse: dubious: !x | !y > > > > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: in > > > > included file > > > (through include/linux/mmzone.h, include/linux/gfp.h, > include/linux/mm.h, ...): > > > > include/linux/page-flags.h:237:46: sparse: sparse: > > > > self-comparison always > > > evaluates to false > > > > include/linux/page-flags.h:237:46: sparse: sparse: > > > > self-comparison always evaluates to false > > > > > > > > vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c > > > ... > > > > 297 for (i = 0; i < config->prd_tbl_num; i++) { > > > > > 298 if (!config->sgls[i] | !config->sgls_nent[i]) > > > > 299 continue; > > > ... > > > > > > If zeros are unlikely the bit-wise 'or' is pretty likely to generate > > > better code than a logical 'or'. > > > > > > David > > > > Good suggestion! > > So the code can be optimized to: > > for (i = 0; i < config->prd_tbl_num; i++) { > > if (config->sgls[i] && config->sgls_nent[i]) { > > ...... > > } > > That just adds a level of indentation to the source. > It'll generate much the same code as !x || !y. > Both will generate two conditional branches. > > It almost certainly makes in immeasurable difference here, but !x | !y can be > generated using (on x86) the 'sete' instruction to get a 0/1 from the x == 0 > compare (that sets the flags), and then doing an 'or' and a single jump. You are right. I do a simple testing: 83 7d f8 00 cmpl $0x0,-0x8(%rbp) 0f 94 c2 sete %dl 83 7d fc 00 cmpl $0x0,-0x4(%rbp) 0f 94 c0 sete %al 09 d0 or %edx,%eax 84 c0 test %al,%al 74 0f je 1186 <main+0x3d> Compare to: 83 7d f8 00 cmpl $0x0,-0x8(%rbp) 74 06 je 116f <main+0x26> 83 7d fc 00 cmpl $0x0,-0x4(%rbp) 75 0f jne 117e <main+0x35> So you suggest I'd better keep the original (!x | !y) code, if just for unlikely Zero comparing? Thanks! Best Regards, Even Xu > > David > > > } > > > > Will create a patch for this. > > > > Best Regards, > > Even Xu > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y 2025-02-06 2:50 ` Xu, Even @ 2025-02-06 9:04 ` David Laight 2025-02-07 6:11 ` Xu, Even 0 siblings, 1 reply; 7+ messages in thread From: David Laight @ 2025-02-06 9:04 UTC (permalink / raw) To: Xu, Even Cc: lkp, oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, Jiri Kosina, Sun, Xinpeng, Srinivas Pandruvada, Mark Pearson On Thu, 6 Feb 2025 02:50:17 +0000 "Xu, Even" <even.xu@intel.com> wrote: > > -----Original Message----- > > From: David Laight <david.laight.linux@gmail.com> > > Sent: Thursday, February 6, 2025 5:57 AM > > To: Xu, Even <even.xu@intel.com> > > Cc: lkp <lkp@intel.com>; oe-kbuild-all@lists.linux.dev; linux- > > kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, Xinpeng > > <xinpeng.sun@intel.com>; Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > > lenovo@squebb.ca> > > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > sparse: dubious: !x | !y > > > > On Wed, 5 Feb 2025 02:56:05 +0000 > > "Xu, Even" <even.xu@intel.com> wrote: > > > > > > -----Original Message----- > > > > From: David Laight <david.laight.linux@gmail.com> > > > > Sent: Thursday, January 30, 2025 6:22 AM > > > > To: lkp <lkp@intel.com> > > > > Cc: Xu, Even <even.xu@intel.com>; oe-kbuild-all@lists.linux.dev; > > > > linux- kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, > > > > Xinpeng <xinpeng.sun@intel.com>; Srinivas Pandruvada > > > > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > > > > lenovo@squebb.ca> > > > > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: > > sparse: > > > > sparse: dubious: !x | !y > > > > > > > > On Wed, 29 Jan 2025 22:02:59 +0800 > > > > kernel test robot <lkp@intel.com> wrote: > > > > > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > master > > > > > head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5 > > > > > commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc-hid: > > > > intel-thc: Add THC DMA interfaces > > > > > date: 3 weeks ago > > > > > config: x86_64-allyesconfig > > > > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFD > > > > > q4ov > > > > > r-lkp@intel.com/config) > > > > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a > > > > > W=1 > > > > > build): > > > > > (https://download.01.org/0day-ci/archive/20250129/202501292144.eFD > > > > > q4ov > > > > > r-lkp@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 <lkp@intel.com> > > > > > | Closes: > > > > > | https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr-lkp@ > > > > > | inte > > > > > | l.com/ > > > > > > > > > > sparse warnings: (new ones prefixed by >>) > > > > > >> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > > > > >> sparse: dubious: !x | !y > > > > > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: in > > > > > included file > > > > (through include/linux/mmzone.h, include/linux/gfp.h, > > include/linux/mm.h, ...): > > > > > include/linux/page-flags.h:237:46: sparse: sparse: > > > > > self-comparison always > > > > evaluates to false > > > > > include/linux/page-flags.h:237:46: sparse: sparse: > > > > > self-comparison always evaluates to false > > > > > > > > > > vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c > > > > ... > > > > > 297 for (i = 0; i < config->prd_tbl_num; i++) { > > > > > > 298 if (!config->sgls[i] | !config->sgls_nent[i]) > > > > > 299 continue; > > > > ... > > > > > > > > If zeros are unlikely the bit-wise 'or' is pretty likely to generate > > > > better code than a logical 'or'. > > > > > > > > David > > > > > > Good suggestion! > > > So the code can be optimized to: > > > for (i = 0; i < config->prd_tbl_num; i++) { > > > if (config->sgls[i] && config->sgls_nent[i]) { > > > ...... > > > } > > > > That just adds a level of indentation to the source. > > It'll generate much the same code as !x || !y. > > Both will generate two conditional branches. > > > > It almost certainly makes in immeasurable difference here, but !x | !y can be > > generated using (on x86) the 'sete' instruction to get a 0/1 from the x == 0 > > compare (that sets the flags), and then doing an 'or' and a single jump. > > You are right. I'm always right (except when I'm not) > I do a simple testing: > > 83 7d f8 00 cmpl $0x0,-0x8(%rbp) > 0f 94 c2 sete %dl > 83 7d fc 00 cmpl $0x0,-0x4(%rbp) > 0f 94 c0 sete %al On a modern cpu those two pairs will run in parallel. > 09 d0 or %edx,%eax > 84 c0 test %al,%al The compiler needn't to add the 'test' instruction, but doesn't really like the fact that the x86 'sete' is only available for 8-bit registers. > 74 0f je 1186 <main+0x3d> > > Compare to: > > 83 7d f8 00 cmpl $0x0,-0x8(%rbp) > 74 06 je 116f <main+0x26> > 83 7d fc 00 cmpl $0x0,-0x4(%rbp) > 75 0f jne 117e <main+0x35> > > So you suggest I'd better keep the original (!x | !y) code, if just for unlikely Zero comparing? Looking at the code I'd guess common case is that both values are non-zero. So in the normal case both tests are needed. The branch predictor will (hopefully) get the tests right after the first iteration, and I suspect the performance of this code isn't that critical. So while using | might be an optimisation (rather than a mistake) changing it to || probably doesn't matter. David > > Thanks! > > Best Regards, > Even Xu > > > > > David > > > > > } > > > > > > Will create a patch for this. > > > > > > Best Regards, > > > Even Xu > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y 2025-02-06 9:04 ` David Laight @ 2025-02-07 6:11 ` Xu, Even 0 siblings, 0 replies; 7+ messages in thread From: Xu, Even @ 2025-02-07 6:11 UTC (permalink / raw) To: David Laight Cc: lkp, oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, Jiri Kosina, Sun, Xinpeng, Srinivas Pandruvada, Mark Pearson > -----Original Message----- > From: David Laight <david.laight.linux@gmail.com> > Sent: Thursday, February 6, 2025 5:05 PM > To: Xu, Even <even.xu@intel.com> > Cc: lkp <lkp@intel.com>; oe-kbuild-all@lists.linux.dev; linux- > kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, Xinpeng > <xinpeng.sun@intel.com>; Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > lenovo@squebb.ca> > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > sparse: dubious: !x | !y > > On Thu, 6 Feb 2025 02:50:17 +0000 > "Xu, Even" <even.xu@intel.com> wrote: > > > > -----Original Message----- > > > From: David Laight <david.laight.linux@gmail.com> > > > Sent: Thursday, February 6, 2025 5:57 AM > > > To: Xu, Even <even.xu@intel.com> > > > Cc: lkp <lkp@intel.com>; oe-kbuild-all@lists.linux.dev; linux- > > > kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; Sun, Xinpeng > > > <xinpeng.sun@intel.com>; Srinivas Pandruvada > > > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > > > lenovo@squebb.ca> > > > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: > sparse: > > > sparse: dubious: !x | !y > > > > > > On Wed, 5 Feb 2025 02:56:05 +0000 > > > "Xu, Even" <even.xu@intel.com> wrote: > > > > > > > > -----Original Message----- > > > > > From: David Laight <david.laight.linux@gmail.com> > > > > > Sent: Thursday, January 30, 2025 6:22 AM > > > > > To: lkp <lkp@intel.com> > > > > > Cc: Xu, Even <even.xu@intel.com>; oe-kbuild-all@lists.linux.dev; > > > > > linux- kernel@vger.kernel.org; Jiri Kosina <jikos@kernel.org>; > > > > > Sun, Xinpeng <xinpeng.sun@intel.com>; Srinivas Pandruvada > > > > > <srinivas.pandruvada@linux.intel.com>; Mark Pearson <mpearson- > > > > > lenovo@squebb.ca> > > > > > Subject: Re: drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: > > > sparse: > > > > > sparse: dubious: !x | !y > > > > > > > > > > On Wed, 29 Jan 2025 22:02:59 +0800 kernel test robot > > > > > <lkp@intel.com> wrote: > > > > > > > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > > master > > > > > > head: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5 > > > > > > commit: a688404b2e20f00cce6d0a2b888ef4ca9154e144 HID: intel-thc- > hid: > > > > > intel-thc: Add THC DMA interfaces > > > > > > date: 3 weeks ago > > > > > > config: x86_64-allyesconfig > > > > > > (https://download.01.org/0day-ci/archive/20250129/202501292144 > > > > > > .eFD > > > > > > q4ov > > > > > > r-lkp@intel.com/config) > > > > > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is > > > > > > a > > > > > > W=1 > > > > > > build): > > > > > > (https://download.01.org/0day-ci/archive/20250129/202501292144 > > > > > > .eFD > > > > > > q4ov > > > > > > r-lkp@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 <lkp@intel.com> > > > > > > | Closes: > > > > > > | https://lore.kernel.org/oe-kbuild-all/202501292144.eFDq4ovr- > > > > > > | lkp@ > > > > > > | inte > > > > > > | l.com/ > > > > > > > > > > > > sparse warnings: (new ones prefixed by >>) > > > > > > >> drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: > > > > > > >> sparse: dubious: !x | !y > > > > > > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c: note: > > > > > > in included file > > > > > (through include/linux/mmzone.h, include/linux/gfp.h, > > > include/linux/mm.h, ...): > > > > > > include/linux/page-flags.h:237:46: sparse: sparse: > > > > > > self-comparison always > > > > > evaluates to false > > > > > > include/linux/page-flags.h:237:46: sparse: sparse: > > > > > > self-comparison always evaluates to false > > > > > > > > > > > > vim +298 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c > > > > > ... > > > > > > 297 for (i = 0; i < config->prd_tbl_num; i++) { > > > > > > > 298 if (!config->sgls[i] | !config->sgls_nent[i]) > > > > > > 299 continue; > > > > > ... > > > > > > > > > > If zeros are unlikely the bit-wise 'or' is pretty likely to > > > > > generate better code than a logical 'or'. > > > > > > > > > > David > > > > > > > > Good suggestion! > > > > So the code can be optimized to: > > > > for (i = 0; i < config->prd_tbl_num; i++) { > > > > if (config->sgls[i] && config->sgls_nent[i]) { > > > > ...... > > > > } > > > > > > That just adds a level of indentation to the source. > > > It'll generate much the same code as !x || !y. > > > Both will generate two conditional branches. > > > > > > It almost certainly makes in immeasurable difference here, but !x | > > > !y can be generated using (on x86) the 'sete' instruction to get a > > > 0/1 from the x == 0 compare (that sets the flags), and then doing an 'or' and a > single jump. > > > > You are right. > > I'm always right (except when I'm not) > > > I do a simple testing: > > > > 83 7d f8 00 cmpl $0x0,-0x8(%rbp) > > 0f 94 c2 sete %dl > > 83 7d fc 00 cmpl $0x0,-0x4(%rbp) > > 0f 94 c0 sete %al > > On a modern cpu those two pairs will run in parallel. > > > 09 d0 or %edx,%eax > > 84 c0 test %al,%al > > The compiler needn't to add the 'test' instruction, but doesn't really like the fact > that the x86 'sete' is only available for 8-bit registers. > > > 74 0f je 1186 <main+0x3d> > > > > Compare to: > > > > 83 7d f8 00 cmpl $0x0,-0x8(%rbp) > > 74 06 je 116f <main+0x26> > > 83 7d fc 00 cmpl $0x0,-0x4(%rbp) > > 75 0f jne 117e <main+0x35> > > > > So you suggest I'd better keep the original (!x | !y) code, if just for unlikely Zero > comparing? > > Looking at the code I'd guess common case is that both values are non-zero. > So in the normal case both tests are needed. > > The branch predictor will (hopefully) get the tests right after the first iteration, > and I suspect the performance of this code isn't that critical. > > So while using | might be an optimisation (rather than a mistake) changing it to || > probably doesn't matter. > > David > Understood. Thanks for your suggestions and so many details! Then I prefer to change back to "||" to make it more readable and avoid miss understanding. Thanks! Best Regards, Even > > > > > Thanks! > > > > Best Regards, > > Even Xu > > > > > > > > David > > > > > > > } > > > > > > > > Will create a patch for this. > > > > > > > > Best Regards, > > > > Even Xu > > > > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-02-07 6:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-29 14:02 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:298:38: sparse: sparse: dubious: !x | !y kernel test robot 2025-01-29 22:21 ` David Laight 2025-02-05 2:56 ` Xu, Even 2025-02-05 21:56 ` David Laight 2025-02-06 2:50 ` Xu, Even 2025-02-06 9:04 ` David Laight 2025-02-07 6:11 ` Xu, Even
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox