All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH] __div64_32: implement division by multiplication for 32-bit arches
Date: Thu, 29 Oct 2015 07:34:23 +0000	[thread overview]
Message-ID: <1446104062.2584.8.camel@synopsys.com> (raw)
In-Reply-To: <alpine.LFD.2.20.1510281907170.630@knanqh.ubzr>

Hi Nicolas,

On Wed, 2015-10-28@19:32 -0400, Nicolas Pitre wrote:
> On Thu, 29 Oct 2015, Alexey Brodkin wrote:
> 
> > Fortunately we already have much better __div64_32() for 32-bit ARM.
> > There in case of division by constant preprocessor calculates so-called
> > "magic number" which is later used in multiplications instead of divisions.
> 
> It's not magic, it is science.  :-)

Indeed, but I was under impression that's how people call that value in that
particular case. So for me it looks appropriate here.

> > It's really nice and very optimal but obviously works only for ARM
> > because ARM assembly is involved.
> > 
> > Now why don't we extend the same approach to all other 32-bit arches
> > with multiplication part implemented in pure C. With good compiler
> > resulting assembly will be quite close to manually written assembly.
> 
> You appear to have left out optimizations where there is no overflow to 
> carry.  That, too, can be determined at compile time.

That might be the case - let me look at that a bit more.
But that was not the biggest problem. I actually wanted to send it
as RFC but due to last minute change I made "git pathc-format -1" and
forgot to change topic from PATCH to RFC.

> > But there's at least 1 problem which I don't know how to solve.
> > Preprocessor magic only happens if __div64_32() is inlined (that's
> > obvious - preprocessor has to know if divider is constant or not).
> > 
> > But __div64_32() is already marked as weak function (which in its turn
> > is required to allow some architectures to provide its own optimal
> > implementations). I.e. addition of "inline" for __div64_32() is not an
> > option.
> 
> You can't inline __div64_32().  It should remain as is and used only for 
> the slow path.
> 
> For the constant based optimization to work, you need to modify do_div() 
> in include/asm-generic/div64.h directly.

I thought about that but if I replace existing implementation of do_div()
with proposed here some arches like SH and MIPS won't be able to use their
own __div64_32() in do_div() any longer.

So how to deal with that then?

> > So I do want to hear opinions on how to proceed with that patch.
> > Indeed there's the simplest solution - use this implementation only in
> > my architecture of preference (read ARC) but IMHO this change may
> > benefit other architectures as well.
> > 
> > Signed-off-by: Alexey Brodkin <abrodkin at synopsys.com>
> > Cc: linux-snps-arc at lists.infradead.org
> > Cc: Vineet Gupta <vgupta at synopsys.com>
> > Cc: Ingo Molnar <mingo at elte.hu>
> > Cc: Stephen Hemminger <shemminger at linux-foundation.org>
> > Cc: David S. Miller <davem at davemloft.net>
> > Cc: Nicolas Pitre <nico at cam.org>
> 
> This email address has been unused for the last 7 years. Please update 
> your reference.

My bad - I blindly took that email from your prehistoric patch
"[ARM] 3611/4: optimize do_div() when divisor is constant".

-Alexey

WARNING: multiple messages have this Message-ID (diff)
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: "nicolas.pitre@linaro.org" <nicolas.pitre@linaro.org>
Cc: "rmk+kernel@arm.linux.org.uk" <rmk+kernel@arm.linux.org.uk>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Vineet.Gupta1@synopsys.com" <Vineet.Gupta1@synopsys.com>,
	"shemminger@linux-foundation.org"
	<shemminger@linux-foundation.org>,
	"mingo@elte.hu" <mingo@elte.hu>,
	"linux-snps-arc@lists.infradead.org" 
	<linux-snps-arc@lists.infradead.org>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH] __div64_32: implement division by multiplication for 32-bit arches
Date: Thu, 29 Oct 2015 07:34:23 +0000	[thread overview]
Message-ID: <1446104062.2584.8.camel@synopsys.com> (raw)
In-Reply-To: <alpine.LFD.2.20.1510281907170.630@knanqh.ubzr>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3083 bytes --]

Hi Nicolas,

On Wed, 2015-10-28 at 19:32 -0400, Nicolas Pitre wrote:
> On Thu, 29 Oct 2015, Alexey Brodkin wrote:
> 
> > Fortunately we already have much better __div64_32() for 32-bit ARM.
> > There in case of division by constant preprocessor calculates so-called
> > "magic number" which is later used in multiplications instead of divisions.
> 
> It's not magic, it is science.  :-)

Indeed, but I was under impression that's how people call that value in that
particular case. So for me it looks appropriate here.

> > It's really nice and very optimal but obviously works only for ARM
> > because ARM assembly is involved.
> > 
> > Now why don't we extend the same approach to all other 32-bit arches
> > with multiplication part implemented in pure C. With good compiler
> > resulting assembly will be quite close to manually written assembly.
> 
> You appear to have left out optimizations where there is no overflow to 
> carry.  That, too, can be determined at compile time.

That might be the case - let me look at that a bit more.
But that was not the biggest problem. I actually wanted to send it
as RFC but due to last minute change I made "git pathc-format -1" and
forgot to change topic from PATCH to RFC.

> > But there's at least 1 problem which I don't know how to solve.
> > Preprocessor magic only happens if __div64_32() is inlined (that's
> > obvious - preprocessor has to know if divider is constant or not).
> > 
> > But __div64_32() is already marked as weak function (which in its turn
> > is required to allow some architectures to provide its own optimal
> > implementations). I.e. addition of "inline" for __div64_32() is not an
> > option.
> 
> You can't inline __div64_32().  It should remain as is and used only for 
> the slow path.
> 
> For the constant based optimization to work, you need to modify do_div() 
> in include/asm-generic/div64.h directly.

I thought about that but if I replace existing implementation of do_div()
with proposed here some arches like SH and MIPS won't be able to use their
own __div64_32() in do_div() any longer.

So how to deal with that then?

> > So I do want to hear opinions on how to proceed with that patch.
> > Indeed there's the simplest solution - use this implementation only in
> > my architecture of preference (read ARC) but IMHO this change may
> > benefit other architectures as well.
> > 
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: linux-snps-arc@lists.infradead.org
> > Cc: Vineet Gupta <vgupta@synopsys.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Stephen Hemminger <shemminger@linux-foundation.org>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Nicolas Pitre <nico@cam.org>
> 
> This email address has been unused for the last 7 years. Please update 
> your reference.

My bad - I blindly took that email from your prehistoric patch
"[ARM] 3611/4: optimize do_div() when divisor is constant".

-Alexeyÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

  reply	other threads:[~2015-10-29  7:34 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-28 22:47 [PATCH] __div64_32: implement division by multiplication for 32-bit arches Alexey Brodkin
2015-10-28 22:47 ` Alexey Brodkin
2015-10-28 23:32 ` Nicolas Pitre
2015-10-28 23:32   ` Nicolas Pitre
2015-10-29  7:34   ` Alexey Brodkin [this message]
2015-10-29  7:34     ` Alexey Brodkin
2015-10-30  1:26   ` Nicolas Pitre
2015-10-30  1:26     ` Nicolas Pitre
2015-10-30  5:41     ` Vineet Gupta
2015-10-30  5:41       ` Vineet Gupta
2015-10-30 12:41       ` Måns Rullgård
2015-10-30 12:41         ` Måns Rullgård
2015-10-30 12:40     ` Måns Rullgård
2015-10-30 12:40       ` Måns Rullgård
2015-10-30 15:17       ` Nicolas Pitre
2015-10-30 15:17         ` Nicolas Pitre
2015-10-30 15:54         ` Alexey Brodkin
2015-10-30 15:54           ` Alexey Brodkin
2015-10-30 16:55           ` Nicolas Pitre
2015-10-30 16:55             ` Nicolas Pitre
2015-10-30 17:45             ` Måns Rullgård
2015-10-30 17:45               ` Måns Rullgård
2015-11-04 23:46               ` Nicolas Pitre
2015-11-04 23:46                 ` Nicolas Pitre
2015-11-04 23:48                 ` Nicolas Pitre
2015-11-04 23:48                   ` Nicolas Pitre
2015-11-05  3:13                   ` Vineet Gupta
2015-11-05  3:13                     ` Vineet Gupta
2015-11-05  5:06                     ` Nicolas Pitre
2015-11-05  5:06                       ` Nicolas Pitre
2015-11-04 23:49                 ` Måns Rullgård
2015-11-04 23:49                   ` Måns Rullgård
2015-10-30 14:28     ` Alexey Brodkin
2015-10-30 14:28       ` Alexey Brodkin
2015-10-29  0:36 ` kbuild test robot
2015-10-29  0:36   ` kbuild test robot
2015-10-29 12:52 ` Måns Rullgård
2015-10-29 12:52   ` Måns Rullgård
2015-10-29 13:05   ` Alexey Brodkin
2015-10-29 13:05     ` Alexey Brodkin
2015-10-29 13:37     ` Måns Rullgård
2015-10-29 13:37       ` Måns Rullgård
2015-10-29 13:31 ` Russell King - ARM Linux
2015-10-29 13:31   ` Russell King - ARM Linux
2015-10-29 14:32   ` Alexey Brodkin
2015-10-29 14:32     ` Alexey Brodkin
2015-10-29 17:09 ` Randy Dunlap
2015-10-29 17:09   ` Randy Dunlap

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=1446104062.2584.8.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=linux-snps-arc@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.