| Message ID | 20240325105823.1483-1-jszhang@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0a16a172879012c42f55ae8c2883e17c1e4e388f |
| Headers | show |
| Series | [v3,RESEND] riscv: select ARCH_HAS_FAST_MULTIPLIER | expand |
On Mon, 25 Mar 2024 03:58:23 PDT (-0700), jszhang@kernel.org wrote: > Currently, riscv linux requires at least IMA, so all platforms have a > multiplier. And I assume the 'mul' efficiency is comparable or better > than a sequence of five or so register-dependent arithmetic > instructions. Select ARCH_HAS_FAST_MULTIPLIER to get slightly nicer > codegen. Refer to commit f9b4192923fa ("[PATCH] bitops: hweight() > speedup") for more details. > > In a simple benchmark test calling hweight64() in a loop, it got: > about 14% performance improvement on JH7110, tested on Milkv Mars. > > about 23% performance improvement on TH1520 and SG2042, tested on > Sipeed LPI4A and SG2042 platform. > > a slight performance drop on CV1800B, tested on milkv duo. Among all > riscv platforms in my hands, this is the only one which sees a slight > performance drop. It means the 'mul' isn't quick enough. However, the > situation exists on x86 too, for example, P4 doesn't have fast > integer multiplies as said in the above commit, x86 also selects > ARCH_HAS_FAST_MULTIPLIER. So let's select ARCH_HAS_FAST_MULTIPLIER > which can benefit almost riscv platforms. > > Samuel also provided some performance numbers: > On Unmatched: 20% speedup for __sw_hweight32 and 30% speedup for > __sw_hweight64. > On D1: 8% speedup for __sw_hweight32 and 8% slowdown for > __sw_hweight64. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > Reviewed-by: Samuel Holland <samuel.holland@sifive.com> > Tested-by: Samuel Holland <samuel.holland@sifive.com> > Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > Hi Palmer, > > Similar as the pgprot_nx patch, this patch missed two merge window too. > Feel free to ask me questions if there's something need to be done from > my side. Sorry I missed these. I know they look small and simple, but they're the sort of patches that have wide-reaching implications and thus just take a long time to review for a tiny diff. I think they just got lost in the shuffle, luckily Alex and Andrea picked up some reviews which helps a ton. Really the best thing to do if you stuff merged is to go review patches that are in front of you in the patchwork queue, as that's what blocks things from getting merged. I know that's not the most fun of answers... I picked these up for the tester, the code is pretty simple so hopefully everything's OK and they'll show up on for-next proper within a day. > > Thanks > > since v2: > - rebase on v6.8-rc1 > - collect Reviewed-by and Tested-by tag > > since v1: > - fix typo in commit msg > - add some performance numbers provided by Samuel > - collect Reviewed-by and Tested-by tag > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index be09c8836d56..aba42b2bf660 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -23,6 +23,7 @@ config RISCV > select ARCH_HAS_DEBUG_VIRTUAL if MMU > select ARCH_HAS_DEBUG_VM_PGTABLE > select ARCH_HAS_DEBUG_WX > + select ARCH_HAS_FAST_MULTIPLIER > select ARCH_HAS_FORTIFY_SOURCE > select ARCH_HAS_GCOV_PROFILE_ALL > select ARCH_HAS_GIGANTIC_PAGE
Hello: This patch was applied to riscv/linux.git (for-next) by Palmer Dabbelt <palmer@rivosinc.com>: On Mon, 25 Mar 2024 18:58:23 +0800 you wrote: > Currently, riscv linux requires at least IMA, so all platforms have a > multiplier. And I assume the 'mul' efficiency is comparable or better > than a sequence of five or so register-dependent arithmetic > instructions. Select ARCH_HAS_FAST_MULTIPLIER to get slightly nicer > codegen. Refer to commit f9b4192923fa ("[PATCH] bitops: hweight() > speedup") for more details. > > [...] Here is the summary with links: - [v3,RESEND] riscv: select ARCH_HAS_FAST_MULTIPLIER https://git.kernel.org/riscv/c/0a16a1728790 You are awesome, thank you!
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index be09c8836d56..aba42b2bf660 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -23,6 +23,7 @@ config RISCV select ARCH_HAS_DEBUG_VIRTUAL if MMU select ARCH_HAS_DEBUG_VM_PGTABLE select ARCH_HAS_DEBUG_WX + select ARCH_HAS_FAST_MULTIPLIER select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE
Currently, riscv linux requires at least IMA, so all platforms have a multiplier. And I assume the 'mul' efficiency is comparable or better than a sequence of five or so register-dependent arithmetic instructions. Select ARCH_HAS_FAST_MULTIPLIER to get slightly nicer codegen. Refer to commit f9b4192923fa ("[PATCH] bitops: hweight() speedup") for more details. In a simple benchmark test calling hweight64() in a loop, it got: about 14% performance improvement on JH7110, tested on Milkv Mars. about 23% performance improvement on TH1520 and SG2042, tested on Sipeed LPI4A and SG2042 platform. a slight performance drop on CV1800B, tested on milkv duo. Among all riscv platforms in my hands, this is the only one which sees a slight performance drop. It means the 'mul' isn't quick enough. However, the situation exists on x86 too, for example, P4 doesn't have fast integer multiplies as said in the above commit, x86 also selects ARCH_HAS_FAST_MULTIPLIER. So let's select ARCH_HAS_FAST_MULTIPLIER which can benefit almost riscv platforms. Samuel also provided some performance numbers: On Unmatched: 20% speedup for __sw_hweight32 and 30% speedup for __sw_hweight64. On D1: 8% speedup for __sw_hweight32 and 8% slowdown for __sw_hweight64. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> Reviewed-by: Samuel Holland <samuel.holland@sifive.com> Tested-by: Samuel Holland <samuel.holland@sifive.com> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+) Hi Palmer, Similar as the pgprot_nx patch, this patch missed two merge window too. Feel free to ask me questions if there's something need to be done from my side. Thanks since v2: - rebase on v6.8-rc1 - collect Reviewed-by and Tested-by tag since v1: - fix typo in commit msg - add some performance numbers provided by Samuel - collect Reviewed-by and Tested-by tag