int_sqrt() adjustments

int_sqrt() adjustments

[Florian La Roche]

__fls() is returning an unsigned long, but fls() and fls64() are
both returning a (signed) int.
As we need a signed int as right operand of "<<" (as Linus pointed out),
change __fls() to fls() for 32bit and also adjust masking the lowest bit
to be a signed int.
Now the 32bit and the 64bit version are again similar.

best regards,

Florian La Roche



Signed-off-by: Florian La Roche <Florian.LaRoche@googlemail.com>
---
 lib/int_sqrt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
index 30e0f9770f88..66eb93105812 100644
--- a/lib/int_sqrt.c
+++ b/lib/int_sqrt.c
@@ -23,7 +23,7 @@ unsigned long int_sqrt(unsigned long x)
 	if (x <= 1)
 		return x;
 
-	m = 1UL << (__fls(x) & ~1UL);
+	m = 1UL << ((fls(x) - 1) & ~1);
 	while (m != 0) {
 		b = y + m;
 		y >>= 1;
@@ -52,7 +52,7 @@ u32 int_sqrt64(u64 x)
 	if (x <= ULONG_MAX)
 		return int_sqrt((unsigned long) x);
 
-	m = 1ULL << ((fls64(x) - 1) & ~1ULL);
+	m = 1ULL << ((fls64(x) - 1) & ~1);
 	while (m != 0) {
 		b = y + m;
 		y >>= 1;
-- 
2.17.1

Re: int_sqrt() adjustments

[Linus Torvalds]

On Fri, Jan 25, 2019 at 7:58 AM Florian La Roche
<florian.laroche@googlemail.com> wrote:
>
> __fls() is returning an unsigned long, but fls() and fls64() are
> both returning a (signed) int.
> As we need a signed int as right operand of "<<" (as Linus pointed out),

It's not that the "signed" part is all that important, it's that using
another type is unnecessary and maybe misleading, when just the
default plain regular integer constant works right.

So I think this:

> -       m = 1UL << (__fls(x) & ~1UL);
> +       m = 1UL << ((fls(x) - 1) & ~1);
..
> -       m = 1ULL << ((fls64(x) - 1) & ~1ULL);
> +       m = 1ULL << ((fls64(x) - 1) & ~1);

is all good and simplifies the code to not have suffixes that don't
really matter or help.

Thanks,

            Linus

Re: int_sqrt() adjustments

[Florian La Roche]

Hello all,

The first part of this patch is wrong: it changes from an unsigned long
param to __fls() to an unsigned int param in fls(). One option would be
to add another implementation of flsl() or given the minimalistic usage
of int_sqrt() in the kernel to keep the current code with __fls() and only
change masking the lowest bit.

The only other nitpick I have come up with: Adding __attribute_const__
could be possible.

best regards,

Florian La Roche

int_sqrt() adjustments

[Florian La Roche]

__fls() is returning an unsigned long, but fls() and fls64() are
both returning a (signed) int.
As we need a signed int as right operand of "<<" (as Linus pointed out),
change __fls() to fls() for 32bit and also adjust masking the lowest bit
to be a signed int.
Now the 32bit and the 64bit version are again similar.

best regards,

Florian La Roche



Signed-off-by: Florian La Roche <Florian.LaRoche@googlemail.com>
---
 lib/int_sqrt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
index 30e0f9770f88..66eb93105812 100644
--- a/lib/int_sqrt.c
+++ b/lib/int_sqrt.c
@@ -23,7 +23,7 @@ unsigned long int_sqrt(unsigned long x)
 	if (x <= 1)
 		return x;
 
-	m = 1UL << (__fls(x) & ~1UL);
+	m = 1UL << ((fls(x) - 1) & ~1L);
 	while (m != 0) {
 		b = y + m;
 		y >>= 1;
@@ -52,7 +52,7 @@ u32 int_sqrt64(u64 x)
 	if (x <= ULONG_MAX)
 		return int_sqrt((unsigned long) x);
 
-	m = 1ULL << ((fls64(x) - 1) & ~1ULL);
+	m = 1ULL << ((fls64(x) - 1) & ~1LL);
 	while (m != 0) {
 		b = y + m;
 		y >>= 1;
-- 
2.17.1