Re: [PATCH] lib: Avoid redundant sizeof checking in __bitmap_weight() calculation.
On Tue, Nov 14, 2017 at 1:23 PM, Rasmus Villemoeswrote: > > hint: sizeof() very rarely evaluates to zero... So this is the same as > "is32 = 1". So the patch as-is is broken (and may explain the 1-byte > delta in vmlinux). But even if this condition is fixed, the patch > doesn't change anything, since the sizeof() evaluation is done at > compile-time, regardless of whether it happens inside the inlined > hweight_long or outside. So it is certainly not worth it to duplicate > the loop. > Right, no need to duplicate the loop. Checking the objdump output, it didn't changed anything at all. Fixed condition nullifies the vmlinux size advantage also. Drop it. Thanks, Rakib.
Re: [PATCH] lib: Avoid redundant sizeof checking in __bitmap_weight() calculation.
On Tue, Nov 14, 2017 at 1:23 PM, Rasmus Villemoes wrote: > > hint: sizeof() very rarely evaluates to zero... So this is the same as > "is32 = 1". So the patch as-is is broken (and may explain the 1-byte > delta in vmlinux). But even if this condition is fixed, the patch > doesn't change anything, since the sizeof() evaluation is done at > compile-time, regardless of whether it happens inside the inlined > hweight_long or outside. So it is certainly not worth it to duplicate > the loop. > Right, no need to duplicate the loop. Checking the objdump output, it didn't changed anything at all. Fixed condition nullifies the vmlinux size advantage also. Drop it. Thanks, Rakib.
Re: [PATCH] lib: Avoid redundant sizeof checking in __bitmap_weight() calculation.
On 14 November 2017 at 07:57, Rakib Mullickwrote: > Currently, during __bitmap_weight() calculation hweight_long() is used. > Inside a hweight_long() a check has been made to figure out whether a > hweight32() or hweight64() version to use. > > diff --git a/lib/bitmap.c b/lib/bitmap.c > index d8f0c09..552096f 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -241,10 +241,15 @@ EXPORT_SYMBOL(__bitmap_subset); > int __bitmap_weight(const unsigned long *bitmap, unsigned int bits) > { > unsigned int k, lim = bits/BITS_PER_LONG; > - int w = 0; > - > - for (k = 0; k < lim; k++) > - w += hweight_long(bitmap[k]); > + int w = 0, is32 = sizeof(bitmap[0]) ? 1 : 0; > + hint: sizeof() very rarely evaluates to zero... So this is the same as "is32 = 1". So the patch as-is is broken (and may explain the 1-byte delta in vmlinux). But even if this condition is fixed, the patch doesn't change anything, since the sizeof() evaluation is done at compile-time, regardless of whether it happens inside the inlined hweight_long or outside. So it is certainly not worth it to duplicate the loop. Rasmus
Re: [PATCH] lib: Avoid redundant sizeof checking in __bitmap_weight() calculation.
On 14 November 2017 at 07:57, Rakib Mullick wrote: > Currently, during __bitmap_weight() calculation hweight_long() is used. > Inside a hweight_long() a check has been made to figure out whether a > hweight32() or hweight64() version to use. > > diff --git a/lib/bitmap.c b/lib/bitmap.c > index d8f0c09..552096f 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -241,10 +241,15 @@ EXPORT_SYMBOL(__bitmap_subset); > int __bitmap_weight(const unsigned long *bitmap, unsigned int bits) > { > unsigned int k, lim = bits/BITS_PER_LONG; > - int w = 0; > - > - for (k = 0; k < lim; k++) > - w += hweight_long(bitmap[k]); > + int w = 0, is32 = sizeof(bitmap[0]) ? 1 : 0; > + hint: sizeof() very rarely evaluates to zero... So this is the same as "is32 = 1". So the patch as-is is broken (and may explain the 1-byte delta in vmlinux). But even if this condition is fixed, the patch doesn't change anything, since the sizeof() evaluation is done at compile-time, regardless of whether it happens inside the inlined hweight_long or outside. So it is certainly not worth it to duplicate the loop. Rasmus
[PATCH] lib: Avoid redundant sizeof checking in __bitmap_weight() calculation.
Currently, during __bitmap_weight() calculation hweight_long() is used. Inside a hweight_long() a check has been made to figure out whether a hweight32() or hweight64() version to use. However, it's unnecessary to do it in case of __bitmap_weight() calculation inside the loop. We can detect whether to use hweight32() or hweight64() upfront and call respective function directly. It also reduces the vmlinux size. Before the patch: textdata bss dec hex filename 129013327798930 1454181635242078219c05e vmlinux After the patch: textdata bss dec hex filename 129013317798930 1454181635242077219c05d vmlinux Signed-off-by: Rakib MullickCc: Andrew Morton Cc: Rasmus Villemoes Cc: Matthew Wilcox Cc: Yury Norov Cc: Mauro Carvalho Chehab --- Patch was created against torvald's tree (commit 43ff2f4db9d0f764). lib/bitmap.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/bitmap.c b/lib/bitmap.c index d8f0c09..552096f 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -241,10 +241,15 @@ EXPORT_SYMBOL(__bitmap_subset); int __bitmap_weight(const unsigned long *bitmap, unsigned int bits) { unsigned int k, lim = bits/BITS_PER_LONG; - int w = 0; - - for (k = 0; k < lim; k++) - w += hweight_long(bitmap[k]); + int w = 0, is32 = sizeof(bitmap[0]) ? 1 : 0; + + if (is32) { + for (k = 0; k < lim; k++) + w += hweight32(bitmap[k]); + } else { + for (k = 0; k < lim; k++) + w += hweight64(bitmap[k]); + } if (bits % BITS_PER_LONG) w += hweight_long(bitmap[k] & BITMAP_LAST_WORD_MASK(bits)); -- 2.9.3
[PATCH] lib: Avoid redundant sizeof checking in __bitmap_weight() calculation.
Currently, during __bitmap_weight() calculation hweight_long() is used. Inside a hweight_long() a check has been made to figure out whether a hweight32() or hweight64() version to use. However, it's unnecessary to do it in case of __bitmap_weight() calculation inside the loop. We can detect whether to use hweight32() or hweight64() upfront and call respective function directly. It also reduces the vmlinux size. Before the patch: textdata bss dec hex filename 129013327798930 1454181635242078219c05e vmlinux After the patch: textdata bss dec hex filename 129013317798930 1454181635242077219c05d vmlinux Signed-off-by: Rakib Mullick Cc: Andrew Morton Cc: Rasmus Villemoes Cc: Matthew Wilcox Cc: Yury Norov Cc: Mauro Carvalho Chehab --- Patch was created against torvald's tree (commit 43ff2f4db9d0f764). lib/bitmap.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/bitmap.c b/lib/bitmap.c index d8f0c09..552096f 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -241,10 +241,15 @@ EXPORT_SYMBOL(__bitmap_subset); int __bitmap_weight(const unsigned long *bitmap, unsigned int bits) { unsigned int k, lim = bits/BITS_PER_LONG; - int w = 0; - - for (k = 0; k < lim; k++) - w += hweight_long(bitmap[k]); + int w = 0, is32 = sizeof(bitmap[0]) ? 1 : 0; + + if (is32) { + for (k = 0; k < lim; k++) + w += hweight32(bitmap[k]); + } else { + for (k = 0; k < lim; k++) + w += hweight64(bitmap[k]); + } if (bits % BITS_PER_LONG) w += hweight_long(bitmap[k] & BITMAP_LAST_WORD_MASK(bits)); -- 2.9.3