public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: David Desobry <david.desobry@formalgen.com>
Cc: tglx@kernel.org, mingo@redhat.com, bp@alien8.de,
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] x86/lib: Optimize num_digits() and fix INT_MIN overflow
Date: Tue, 20 Jan 2026 21:46:12 +0000	[thread overview]
Message-ID: <20260120214612.73b83cbe@pumpkin> (raw)
In-Reply-To: <20260120174748.302078-1-david.desobry@formalgen.com>

On Tue, 20 Jan 2026 18:47:48 +0100
David Desobry <david.desobry@formalgen.com> wrote:

> The current implementation of num_digits() uses a loop with repeated
> multiplication, which is inefficient. Furthermore, the negation of
> the input value "val = -val" causes undefined behavior when val is
> INT_MIN, as its absolute value cannot be represented as a 32-bit integer.
> 
> Replace the loop with a switch statement using GCC case ranges. This
> allows the compiler to generate a jump table or a series of optimized
> comparisons, providing O(1) performance. By using an unsigned int to
> handle the magnitude, we safely handle the INT_MIN case without
> relying on 64-bit types or undefined signed overflow.
> 
> Removed the outdated comment.
> 
> Signed-off-by: David Desobry <david.desobry@formalgen.com>
> ---
> v2:
>  - Replaced loop with switch statement and GCC case ranges.
>  - Fixed INT_MIN overflow using unsigned int cast instead of s64/long long.
>  - Removed outdated comment regarding mobile submission.
>  arch/x86/lib/misc.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c
> index 40b81c338ae5..03ba028d5326 100644
> --- a/arch/x86/lib/misc.c
> +++ b/arch/x86/lib/misc.c
> @@ -3,22 +3,37 @@
>  
>  /*
>   * Count the digits of @val including a possible sign.
> - *
> - * (Typed on and submitted from hpa's mobile phone.)
>   */
>  int num_digits(int val)
>  {
> -	long long m = 10;
> -	int d = 1;
> +	unsigned int v = val;
> +	int d = 0;
>  
>  	if (val < 0) {
> -		d++;
> -		val = -val;
> +		d = 1;
> +		v = -v;
>  	}

Maybe better to write as:
	if (val < 0) {
		d = 1;
		v = -val;
	} else {
		d = 0;
		v = val;
	}

The compiler will only generate one jump.

>  
> -	while (val >= m) {
> -		m *= 10;
> -		d++;
> +	switch (v) {
> +	case 0 ... 9:
> +		return d + 1;
> +	case 10 ... 99:
> +		return d + 2;
> +	case 100 ... 999:
> +		return d + 3;
> +	case 1000 ... 9999:
> +		return d + 4;
> +	case 10000 ... 99999:
> +		return d + 5;
> +	case 100000 ... 999999:
> +		return d + 6;
> +	case 1000000 ... 9999999:
> +		return d + 7;
> +	case 10000000 ... 99999999:
> +		return d + 8;
> +	case 100000000 ... 999999999:
> +		return d + 9;
> +	default:
> +		return d + 10;

clang generates something really horrid for that.

Either:
    if (v <= 9) return d + 1;
    if (v <= 99) return d + 2;
    if (v <= 999) return d + 3;
    if (v <= 9999) return d + 4;
    if (v <= 99999) return d + 5;
    if (v <= 999999) return d + 6;
    if (v <= 9999999) return d + 7;
    if (v <= 99999999) return d + 8;
    if (v <= 999999999) return d + 9;
    return d + 10;
or:
    if ((++d && v > 9) &&
        (++d && v > 99) &&
        (++d && v > 999) &&
        (++d && v > 9999) &&
        (++d && v > 99999) &&
        (++d && v > 999999) &&
        (++d && v > 9999999) &&
        (++d && v > 99999999) &&
        (++d && v > 999999999))
        d++;
    return d;
generate better code.
In particular it is almost certainly best to only have one taken branch.
Dumping in a load of unlikely() might help that as well.
(Neither compiler does the ++d inline, the add is done before the return.)

	David

>  	}
> -	return d;
>  }


  parent reply	other threads:[~2026-01-20 21:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20 17:47 [PATCH v2] x86/lib: Optimize num_digits() and fix INT_MIN overflow David Desobry
2026-01-20 18:03 ` H. Peter Anvin
2026-01-20 21:46 ` David Laight [this message]
2026-01-20 23:19   ` [PATCH v3] " David Desobry
2026-01-20 23:32   ` [PATCH v2] " David Desobry
2026-01-20 23:49     ` H. Peter Anvin
2026-01-21  0:04       ` David Desobry
2026-01-21  0:17         ` H. Peter Anvin
2026-01-21  9:39           ` [PATCH v4] " David Desobry
2026-01-21 11:36             ` David Laight
2026-01-21 15:46               ` H. Peter Anvin
2026-01-21  9:54         ` [PATCH v2] " David Laight
2026-01-21 10:32           ` [PATCH v5] x86/lib: Rename num_digits() to num_digits_u32() and optimize David Desobry
2026-01-21 10:40           ` [PATCH v2] x86/lib: Optimize num_digits() and fix INT_MIN overflow David Desobry
2026-01-21 11:51       ` Maciej W. Rozycki
2026-01-23  7:06         ` H. Peter Anvin
2026-01-23 10:55           ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260120214612.73b83cbe@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=david.desobry@formalgen.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox