2024-05-25 17:45:51

by Thorsten Blum

[permalink] [raw]
Subject: [PATCH] x86/asm/bitops: Change function return types from long to int

Change the return types of bitops functions (ffs, ffz, and fls) from
long to int. The expected return values are in the range [0, 64], for
which int is sufficient.

Additionally, int aligns well with the return types of the corresponding
__builtin_* functions, potentially reducing overall type conversions.

Many of the existing bitops functions already return an int and don't
need to be changed.

With GCC 14 and defconfig, these changes reduced the number of x86-64
instructions by 342 (using objdump -d and compared to 0b32d436c015).

Summary of the most significant changes in instruction counts:

insn before after diff
----------------------------
ja 16044 15166 -878
cmp 171197 170583 -614
movsxd 16796 16271 -525
add 94692 94454 -238
jbe 9053 8815 -238
xor 97499 97478 -21
...
and 47291 47312 +21
nop 1036207 1036233 +26
cs 5622 5656 +34
int3 2815714 2815842 +128
lea 118250 118488 +238
je 190859 191132 +273
test 204887 205484 +597
jne 134316 135161 +845

Signed-off-by: Thorsten Blum <[email protected]>
---
arch/x86/include/asm/bitops.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index b96d45944c59..4d91517a395d 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -246,7 +246,7 @@ arch_test_bit_acquire(unsigned long nr, const volatile unsigned long *addr)
variable_test_bit(nr, addr);
}

-static __always_inline unsigned long variable__ffs(unsigned long word)
+static __always_inline unsigned int variable__ffs(unsigned long word)
{
asm("rep; bsf %1,%0"
: "=r" (word)
@@ -262,10 +262,10 @@ static __always_inline unsigned long variable__ffs(unsigned long word)
*/
#define __ffs(word) \
(__builtin_constant_p(word) ? \
- (unsigned long)__builtin_ctzl(word) : \
+ (unsigned int)__builtin_ctzl(word) : \
variable__ffs(word))

-static __always_inline unsigned long variable_ffz(unsigned long word)
+static __always_inline unsigned int variable_ffz(unsigned long word)
{
asm("rep; bsf %1,%0"
: "=r" (word)
@@ -281,7 +281,7 @@ static __always_inline unsigned long variable_ffz(unsigned long word)
*/
#define ffz(word) \
(__builtin_constant_p(word) ? \
- (unsigned long)__builtin_ctzl(~word) : \
+ (unsigned int)__builtin_ctzl(~word) : \
variable_ffz(word))

/*
@@ -290,7 +290,7 @@ static __always_inline unsigned long variable_ffz(unsigned long word)
*
* Undefined if no set bit exists, so code should check against 0 first.
*/
-static __always_inline unsigned long __fls(unsigned long word)
+static __always_inline unsigned int __fls(unsigned long word)
{
if (__builtin_constant_p(word))
return BITS_PER_LONG - 1 - __builtin_clzl(word);

base-commit: 0b32d436c015d5a88b3368405e3d8fe82f195a54
--
2.45.1



2024-05-25 23:01:17

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] x86/asm/bitops: Change function return types from long to int

Hi Thorsten,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 0b32d436c015d5a88b3368405e3d8fe82f195a54]

url: https://github.com/intel-lab-lkp/linux/commits/Thorsten-Blum/x86-asm-bitops-Change-function-return-types-from-long-to-int/20240526-014828
base: 0b32d436c015d5a88b3368405e3d8fe82f195a54
patch link: https://lore.kernel.org/r/20240525174448.174824-2-thorsten.blum%40toblux.com
patch subject: [PATCH] x86/asm/bitops: Change function return types from long to int
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240526/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240526/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from include/linux/printk.h:573,
from include/asm-generic/bug.h:22,
from arch/x86/include/asm/bug.h:87,
from include/linux/bug.h:5,
from include/linux/fortify-string.h:6,
from include/linux/string.h:374,
from arch/x86/include/asm/page_32.h:18,
from arch/x86/include/asm/page.h:14,
from arch/x86/include/asm/processor.h:20,
from include/linux/sched.h:13,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/devcoredump.h:8,
from sound/soc/intel/catpt/dsp.c:8:
sound/soc/intel/catpt/dsp.c: In function 'catpt_dsp_set_srampge':
>> sound/soc/intel/catpt/dsp.c:181:44: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'u32' {aka 'unsigned int'} [-Wformat=]
181 | dev_dbg(cdev->dev, "sanitize block %ld: off 0x%08x\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:224:29: note: in definition of macro '__dynamic_func_call_cls'
224 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:273:9: note: in expansion of macro '_dynamic_func_call'
273 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:9: note: in expansion of macro 'dynamic_dev_dbg'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:30: note: in expansion of macro 'dev_fmt'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
sound/soc/intel/catpt/dsp.c:181:25: note: in expansion of macro 'dev_dbg'
181 | dev_dbg(cdev->dev, "sanitize block %ld: off 0x%08x\n",
| ^~~~~~~
sound/soc/intel/catpt/dsp.c:181:62: note: format string is defined here
181 | dev_dbg(cdev->dev, "sanitize block %ld: off 0x%08x\n",
| ~~^
| |
| long int
| %d


vim +181 sound/soc/intel/catpt/dsp.c

64b9b1b005743a Cezary Rojewski 2020-09-29 153
ba202a7bc3da05 Cezary Rojewski 2020-09-29 154 static void catpt_dsp_set_srampge(struct catpt_dev *cdev, struct resource *sram,
ba202a7bc3da05 Cezary Rojewski 2020-09-29 155 unsigned long mask, unsigned long new)
ba202a7bc3da05 Cezary Rojewski 2020-09-29 156 {
ba202a7bc3da05 Cezary Rojewski 2020-09-29 157 unsigned long old;
ba202a7bc3da05 Cezary Rojewski 2020-09-29 158 u32 off = sram->start;
ba202a7bc3da05 Cezary Rojewski 2020-09-29 159 u32 b = __ffs(mask);
ba202a7bc3da05 Cezary Rojewski 2020-09-29 160
ba202a7bc3da05 Cezary Rojewski 2020-09-29 161 old = catpt_readl_pci(cdev, VDRTCTL0) & mask;
ba202a7bc3da05 Cezary Rojewski 2020-09-29 162 dev_dbg(cdev->dev, "SRAMPGE [0x%08lx] 0x%08lx -> 0x%08lx",
ba202a7bc3da05 Cezary Rojewski 2020-09-29 163 mask, old, new);
ba202a7bc3da05 Cezary Rojewski 2020-09-29 164
ba202a7bc3da05 Cezary Rojewski 2020-09-29 165 if (old == new)
ba202a7bc3da05 Cezary Rojewski 2020-09-29 166 return;
ba202a7bc3da05 Cezary Rojewski 2020-09-29 167
ba202a7bc3da05 Cezary Rojewski 2020-09-29 168 catpt_updatel_pci(cdev, VDRTCTL0, mask, new);
ba202a7bc3da05 Cezary Rojewski 2020-09-29 169 /* wait for SRAM power gating to propagate */
ba202a7bc3da05 Cezary Rojewski 2020-09-29 170 udelay(60);
ba202a7bc3da05 Cezary Rojewski 2020-09-29 171
ba202a7bc3da05 Cezary Rojewski 2020-09-29 172 /*
ba202a7bc3da05 Cezary Rojewski 2020-09-29 173 * Dummy read as the very first access after block enable
ba202a7bc3da05 Cezary Rojewski 2020-09-29 174 * to prevent byte loss in future operations.
ba202a7bc3da05 Cezary Rojewski 2020-09-29 175 */
ba202a7bc3da05 Cezary Rojewski 2020-09-29 176 for_each_clear_bit_from(b, &new, fls_long(mask)) {
ba202a7bc3da05 Cezary Rojewski 2020-09-29 177 u8 buf[4];
ba202a7bc3da05 Cezary Rojewski 2020-09-29 178
ba202a7bc3da05 Cezary Rojewski 2020-09-29 179 /* newly enabled: new bit=0 while old bit=1 */
ba202a7bc3da05 Cezary Rojewski 2020-09-29 180 if (test_bit(b, &old)) {
ba202a7bc3da05 Cezary Rojewski 2020-09-29 @181 dev_dbg(cdev->dev, "sanitize block %ld: off 0x%08x\n",
ba202a7bc3da05 Cezary Rojewski 2020-09-29 182 b - __ffs(mask), off);
ba202a7bc3da05 Cezary Rojewski 2020-09-29 183 memcpy_fromio(buf, cdev->lpe_ba + off, sizeof(buf));
ba202a7bc3da05 Cezary Rojewski 2020-09-29 184 }
ba202a7bc3da05 Cezary Rojewski 2020-09-29 185 off += CATPT_MEMBLOCK_SIZE;
ba202a7bc3da05 Cezary Rojewski 2020-09-29 186 }
ba202a7bc3da05 Cezary Rojewski 2020-09-29 187 }
ba202a7bc3da05 Cezary Rojewski 2020-09-29 188

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-26 03:56:57

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] x86/asm/bitops: Change function return types from long to int

Hi Thorsten,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 0b32d436c015d5a88b3368405e3d8fe82f195a54]

url: https://github.com/intel-lab-lkp/linux/commits/Thorsten-Blum/x86-asm-bitops-Change-function-return-types-from-long-to-int/20240526-014828
base: 0b32d436c015d5a88b3368405e3d8fe82f195a54
patch link: https://lore.kernel.org/r/20240525174448.174824-2-thorsten.blum%40toblux.com
patch subject: [PATCH] x86/asm/bitops: Change function return types from long to int
config: x86_64-randconfig-121-20240526 (https://download.01.org/0day-ci/archive/20240526/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240526/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)
arch/x86/kvm/emulate.c:5105:21: sparse: sparse: arithmetics on pointers to functions
arch/x86/kvm/emulate.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h, ...):
include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
>> arch/x86/kvm/emulate.c:5105:49: sparse: sparse: non size-preserving pointer to integer cast
>> arch/x86/kvm/emulate.c:5105:49: sparse: sparse: non size-preserving integer to pointer cast

vim +5105 arch/x86/kvm/emulate.c

cbe2c9d30aa69b Avi Kivity 2012-04-09 5099
3009afc6e39e78 Sean Christopherson 2020-01-21 5100 static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop)
e28bbd44dad134 Avi Kivity 2013-01-04 5101 {
e28bbd44dad134 Avi Kivity 2013-01-04 5102 ulong flags = (ctxt->eflags & EFLAGS_MASK) | X86_EFLAGS_IF;
4548f63e651164 Josh Poimboeuf 2016-03-09 5103
b9fa409b00a1ed Avi Kivity 2013-02-09 5104 if (!(ctxt->d & ByteOp))
e28bbd44dad134 Avi Kivity 2013-01-04 @5105 fop += __ffs(ctxt->dst.bytes) * FASTOP_SIZE;
4548f63e651164 Josh Poimboeuf 2016-03-09 5106
1a29b5b7f347a1 Peter Zijlstra 2018-01-25 5107 asm("push %[flags]; popf; " CALL_NOSPEC " ; pushf; pop %[flags]\n"
b8c0b6ae498fe5 Avi Kivity 2013-02-09 5108 : "+a"(ctxt->dst.val), "+d"(ctxt->src.val), [flags]"+D"(flags),
1a29b5b7f347a1 Peter Zijlstra 2018-01-25 5109 [thunk_target]"+S"(fop), ASM_CALL_CONSTRAINT
b8c0b6ae498fe5 Avi Kivity 2013-02-09 5110 : "c"(ctxt->src2.val));
4548f63e651164 Josh Poimboeuf 2016-03-09 5111
e28bbd44dad134 Avi Kivity 2013-01-04 5112 ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);
b8c0b6ae498fe5 Avi Kivity 2013-02-09 5113 if (!fop) /* exception is returned in fop variable */
b8c0b6ae498fe5 Avi Kivity 2013-02-09 5114 return emulate_de(ctxt);
e28bbd44dad134 Avi Kivity 2013-01-04 5115 return X86EMUL_CONTINUE;
e28bbd44dad134 Avi Kivity 2013-01-04 5116 }
dd856efafe6097 Avi Kivity 2012-08-27 5117

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki