From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>
Cc: Jules Bashizi Irenge <jbi.octave@gmail.com>,
"linux-perf-users@vger.kernel.org"
<linux-perf-users@vger.kernel.org>
Subject: RE: [PATCH] perf/x86/amd/power: Use div64_u64 onstead of do_div()
Date: Mon, 6 May 2024 20:25:44 +0000 [thread overview]
Message-ID: <a0bdbe9832cc40e29cd63a71cec95fba@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=wiGUkPz+9gN_uwOfg-VGP4Z5UBKe-xBrn3uHMtJ85UarA@mail.gmail.com>
From: Linus Torvalds
> Sent: 06 May 2024 18:27
>
> On Mon, 6 May 2024 at 09:45, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So we probably _should_ have made "do_div()" be a "64-by-long" divide.
>
> Actually, some of those conversions are just wrong and stupid, and
> didn't have 'long' divisors at all.
>
> And we do have "div64_ul()", although it's not used as widely as it could be.
>
> But I think we can do the reverse, and just teach "div64_u64()" to do
> what gcc should always have done, and look at the divisor range at
> compile-time.
>
> Something like this, perhaps:
>
> /*
> * Do a 64-by-64 divide using div64_ul / div64_long / div64_u64
> * depending on type of divisor.
> */
> #define div64_u64_type(a,b) _Generic((b), \
> unsigned long: div64_ul(a,b), \
> long: div64_long(a,b), \
> int: div64_long(a,b), \
> default: div64_u64(a,b))
>
> /* Is it zero without evaluating it? */
> #define is_constant_zero(x) \
> (__builtin_constant_p(x) && !(x))
>
> /* This catches constants and compile-time ranges */
> #define is_32_bit(x) \
> is_constant_zero((x)>>16>>16)
>
> /*
> * Pick the right division helper based on
> * value or type of the divisor
> */
> #define div64_u64(a,b) \
> (is_32_bit(b) ? do_div64(a,b) : div64_u64_type(a,b))
>
> should now make div64_u64() do the right thing and avoid the 64-bit
> divide for relevant types (the signed type case makes it a bit less
> obvious, but whatever).
I don't think there are any signed divides to worry about.
Signed divides are also too painful to sort out.
You have to worry about all 4 quadrants.
Which is pretty much exactly what I proposed :-)
(apart from the run-time check to avoid calling div64_u64().)
By the time you've done the is_32_bit(b) you've only got
64 bit values left (well and 128 but...), they might be 'long'
or 'long long' on 64bit but that doesn't matter.
I'm not sure of the best way to return a quotient and remainder
on a 32bit system.
The 'user' api is best as:
quotient = divu_xxx(dividend, divisor, "ient);
where divisor and quotient are the same type and quotient
might be NULL - and might be supplied by a wrapper.
Perhaps something like (untested):
#define div64_u64_rem(a, b, rem) ({ \
typeof (b) _q; \
if (is_32_bit(b)) { \
u64 _q_r = _div_u64_u32_rem(a, b);
if (!is_constant_zero(rem))
*rem = _q_r >> 32;
_q = _q_r & 0xffffffff;
} else { \
_q = _divu64_u64_rem(a, b, rem); \
} \
_q; \
})
Then x86 would have a #define/inline for _div_u32_rem().
I suspect the '*rem' needs to be *(typeof(&(b))rem
and something like (0 ? &b : rem) added somewhere.
Perhaps *(typeof(0 ? &b : rem))(rem) DTRT ?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2024-05-06 20:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-28 16:40 [PATCH] perf/x86/amd/power: Use div64_u64 onstead of do_div() Jules Irenge
2024-05-02 16:25 ` David Laight
2024-05-02 16:34 ` Jules Bashizi Irenge
2024-05-02 22:18 ` David Laight
2024-05-02 22:59 ` Linus Torvalds
2024-05-04 22:53 ` David Laight
2024-05-05 19:38 ` Linus Torvalds
2024-05-06 16:39 ` David Laight
2024-05-06 16:45 ` Linus Torvalds
2024-05-06 17:26 ` Linus Torvalds
2024-05-06 20:25 ` David Laight [this message]
2024-05-06 17:43 ` David Laight
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=a0bdbe9832cc40e29cd63a71cec95fba@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=jbi.octave@gmail.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=torvalds@linux-foundation.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).