* [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I @ 2020-07-09 5:19 Chenxi Mao 2020-07-21 1:17 ` Palmer Dabbelt 0 siblings, 1 reply; 7+ messages in thread From: Chenxi Mao @ 2020-07-09 5:19 UTC (permalink / raw) To: paul.walmsley, palmer, aou Cc: linux-riscv, Chenxi Mao, linux-kernel, chenxi.mao2013 Enable ARCH_HAS_FAST_MULTIPLIER on RV64I which works fine on GCC-9.3 and GCC-10.1 PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. Signed-off-by: Chenxi Mao <maochenxi@eswin.com> --- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 128192e14ff2..84e6777fecad 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -202,6 +202,7 @@ config ARCH_RV64I bool "RV64I" select 64BIT select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 + select ARCH_HAS_FAST_MULTIPLIER select HAVE_DYNAMIC_FTRACE if MMU select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD -- 2.25.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I 2020-07-09 5:19 [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I Chenxi Mao @ 2020-07-21 1:17 ` Palmer Dabbelt 2020-07-21 1:47 ` Chenxi Mao 2020-07-23 1:59 ` Chenxi Mao 0 siblings, 2 replies; 7+ messages in thread From: Palmer Dabbelt @ 2020-07-21 1:17 UTC (permalink / raw) To: maochenxi Cc: aou, maochenxi, Paul Walmsley, linux-kernel, chenxi.mao2013, linux-riscv On Wed, 08 Jul 2020 22:19:22 PDT (-0700), maochenxi@eswin.com wrote: > Enable ARCH_HAS_FAST_MULTIPLIER on RV64I > which works fine on GCC-9.3 and GCC-10.1 > > PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. > > Signed-off-by: Chenxi Mao <maochenxi@eswin.com> > --- > arch/riscv/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 128192e14ff2..84e6777fecad 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -202,6 +202,7 @@ config ARCH_RV64I > bool "RV64I" > select 64BIT > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 > + select ARCH_HAS_FAST_MULTIPLIER > select HAVE_DYNAMIC_FTRACE if MMU > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD Ah, thanks -- this one didn't show up when I was looking at the last one. I think we can put the fast multiplier on rv32 and rv64, there shouldn't be any difference there. I guess in theory we should be sticking this all in some sort of "platform type" optimization flags, but that's probably bit much for now. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I 2020-07-21 1:17 ` Palmer Dabbelt @ 2020-07-21 1:47 ` Chenxi Mao 2020-07-21 2:38 ` Chenxi Mao 2020-07-23 1:59 ` Chenxi Mao 1 sibling, 1 reply; 7+ messages in thread From: Chenxi Mao @ 2020-07-21 1:47 UTC (permalink / raw) To: Palmer Dabbelt Cc: chenxi.mao2013, linux-riscv, aou, linux-kernel, Paul Walmsley Hi Palmer: Thanks for your reply. Frankly, I didn't test ARCH_HAS_FAST_MULTIPLIER on RV32, so I cannot put it in RISCV platform. I am trying to comparing ARM64 with Riscv to find out more optimization configurations. I suggest to enable ARCH_HAS_FAST_MULTIPLIER on RV64 first. If someone else evaluate this on RV32, we could move it to RISCV platform. Chenxi On 2020/7/21 上午9:17, Palmer Dabbelt wrote: > On Wed, 08 Jul 2020 22:19:22 PDT (-0700), maochenxi@eswin.com wrote: >> Enable ARCH_HAS_FAST_MULTIPLIER on RV64I >> which works fine on GCC-9.3 and GCC-10.1 >> >> PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. >> >> Signed-off-by: Chenxi Mao <maochenxi@eswin.com> >> --- >> arch/riscv/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 128192e14ff2..84e6777fecad 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -202,6 +202,7 @@ config ARCH_RV64I >> bool "RV64I" >> select 64BIT >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 >> + select ARCH_HAS_FAST_MULTIPLIER >> select HAVE_DYNAMIC_FTRACE if MMU >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >> select HAVE_FTRACE_MCOUNT_RECORD > > Ah, thanks -- this one didn't show up when I was looking at the last one. I > think we can put the fast multiplier on rv32 and rv64, there shouldn't be any > difference there. I guess in theory we should be sticking this all in some > sort of "platform type" optimization flags, but that's probably bit much for > now. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I 2020-07-21 1:47 ` Chenxi Mao @ 2020-07-21 2:38 ` Chenxi Mao 0 siblings, 0 replies; 7+ messages in thread From: Chenxi Mao @ 2020-07-21 2:38 UTC (permalink / raw) To: Palmer Dabbelt Cc: chenxi.mao2013, linux-riscv, aou, linux-kernel, Paul Walmsley Hi Palmer: Move to RISCV platform is ok for me, but I cannot evaluate RV32 condition. Chenxi On 2020/7/21 上午9:47, Chenxi Mao wrote: > Hi Palmer: > > Thanks for your reply. > > Frankly, I didn't test ARCH_HAS_FAST_MULTIPLIER on RV32, > > so I cannot put it in RISCV platform. > > I am trying to comparing ARM64 with Riscv to find out more optimization > > configurations. > > I suggest to enable ARCH_HAS_FAST_MULTIPLIER on RV64 first. > > If someone else evaluate this on RV32, we could move it to RISCV platform. > > > Chenxi > > > On 2020/7/21 上午9:17, Palmer Dabbelt wrote: >> On Wed, 08 Jul 2020 22:19:22 PDT (-0700), maochenxi@eswin.com wrote: >>> Enable ARCH_HAS_FAST_MULTIPLIER on RV64I >>> which works fine on GCC-9.3 and GCC-10.1 >>> >>> PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. >>> >>> Signed-off-by: Chenxi Mao <maochenxi@eswin.com> >>> --- >>> arch/riscv/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >>> index 128192e14ff2..84e6777fecad 100644 >>> --- a/arch/riscv/Kconfig >>> +++ b/arch/riscv/Kconfig >>> @@ -202,6 +202,7 @@ config ARCH_RV64I >>> bool "RV64I" >>> select 64BIT >>> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 >>> + select ARCH_HAS_FAST_MULTIPLIER >>> select HAVE_DYNAMIC_FTRACE if MMU >>> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >>> select HAVE_FTRACE_MCOUNT_RECORD >> Ah, thanks -- this one didn't show up when I was looking at the last one. I >> think we can put the fast multiplier on rv32 and rv64, there shouldn't be any >> difference there. I guess in theory we should be sticking this all in some >> sort of "platform type" optimization flags, but that's probably bit much for >> now. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I 2020-07-21 1:17 ` Palmer Dabbelt 2020-07-21 1:47 ` Chenxi Mao @ 2020-07-23 1:59 ` Chenxi Mao 2020-07-23 2:13 ` Palmer Dabbelt 1 sibling, 1 reply; 7+ messages in thread From: Chenxi Mao @ 2020-07-23 1:59 UTC (permalink / raw) To: Palmer Dabbelt, kernel Cc: aou, wangqiang1, Paul Walmsley, linux-kernel, chenxi.mao2013, linux-riscv Hi Palmer and Emil: As Emil mentioned in previous E-mail loop, I did the same test on my kernel as well. My kernel is based on Linux 5.8-RC6 with GCC-10.1. (ISA C extension enabled) The disassembly code as below: CONFIG_ARCH_HAS_FAST_MULTIPLIER enabled: 0000000000000000 <__sw_hweight32>: 0: 555557b7 lui a5,0x55555 4: 0015571b srliw a4,a0,0x1 8: 55578793 addi a5,a5,1365 # 55555555 <.LASF5+0x5555509d> c: 8ff9 and a5,a5,a4 e: 9d1d subw a0,a0,a5 0000000000000010 <.LVL1>: 10: 333337b7 lui a5,0x33333 14: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332e7b> 18: 0025571b srliw a4,a0,0x2 1c: 8d7d and a0,a0,a5 1e: 8ff9 and a5,a5,a4 20: 9fa9 addw a5,a5,a0 22: 0047d51b srliw a0,a5,0x4 26: 9fa9 addw a5,a5,a0 28: 0f0f1537 lui a0,0xf0f1 2c: 1141 addi sp,sp,-16 2e: f0f50513 addi a0,a0,-241 # f0f0f0f <.LASF5+0xf0f0a57> 32: e422 sd s0,8(sp) 34: 8fe9 and a5,a5,a0 36: 0800 addi s0,sp,16 38: 0087951b slliw a0,a5,0x8 3c: 6422 ld s0,8(sp) 3e: 9d3d addw a0,a0,a5 40: 0105179b slliw a5,a0,0x10 44: 9d3d addw a0,a0,a5 46: 0185551b srliw a0,a0,0x18 4a: 0141 addi sp,sp,16 4c: 8082 ret CONFIG_ARCH_HAS_FAST_MULTIPLIER disabled: 000000000000004e <__sw_hweight32_default>: 4e: 55555737 lui a4,0x55555 52: 0015579b srliw a5,a0,0x1 56: 55570713 addi a4,a4,1365 # 55555555 <.LASF5+0x5555509d> 5a: 8ff9 and a5,a5,a4 5c: 9d1d subw a0,a0,a5 000000000000005e <.LVL3>: 5e: 333337b7 lui a5,0x33333 62: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332e7b> 66: 0025571b srliw a4,a0,0x2 6a: 8d7d and a0,a0,a5 6c: 8ff9 and a5,a5,a4 6e: 9fa9 addw a5,a5,a0 70: 0047d51b srliw a0,a5,0x4 74: 9d3d addw a0,a0,a5 76: 0f0f17b7 lui a5,0xf0f1 7a: 1141 addi sp,sp,-16 7c: f0f78793 addi a5,a5,-241 # f0f0f0f <.LASF5+0xf0f0a57> 80: e422 sd s0,8(sp) 82: 8fe9 and a5,a5,a0 84: 0800 addi s0,sp,16 86: 0087d51b srliw a0,a5,0x8 8a: 6422 ld s0,8(sp) 8c: 9fa9 addw a5,a5,a0 8e: 0107d51b srliw a0,a5,0x10 92: 9d3d addw a0,a0,a5 94: 0ff57513 andi a0,a0,255 98: 0141 addi sp,sp,16 9a: 8082 ret This 2 implementations is almost same but small differences. Especially in CONFIG_ARCH_HAS_FAST_MULTIPLIER condition, below code didn't use "mul" instructions. " return (w * 0x01010101) >> 24; " So I am trying to translate this code with inline assembly as below: //return (w * 0x01010101) >> 24; __asm__ ( " mul %0, %0, %1\n" : "+r" (w) : "r" (w), "r"(0x01010101) :); return w >> 24; After above change, the disassambly as below: 0000000000000000 <__sw_hweight32>: 0: 555557b7 lui a5,0x55555 4: 0015571b srliw a4,a0,0x1 8: 55578793 addi a5,a5,1365 # 55555555 <.LASF5+0x55555119> c: 8ff9 and a5,a5,a4 e: 9d1d subw a0,a0,a5 0000000000000010 <.LVL1>: 10: 333337b7 lui a5,0x33333 14: 0025571b srliw a4,a0,0x2 18: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332ef7> 1c: 8d7d and a0,a0,a5 1e: 8ff9 and a5,a5,a4 20: 9fa9 addw a5,a5,a0 22: 0047d71b srliw a4,a5,0x4 26: 9f3d addw a4,a4,a5 28: 0f0f17b7 lui a5,0xf0f1 2c: 1141 addi sp,sp,-16 2e: f0f78793 addi a5,a5,-241 # f0f0f0f <.LASF5+0xf0f0ad3> 32: e422 sd s0,8(sp) 34: 8ff9 and a5,a5,a4 36: 0800 addi s0,sp,16 38: 01010737 lui a4,0x1010 3c: 853e mv a0,a5 000000000000003e <.LVL2>: 3e: 1017071b addiw a4,a4,257 42: 02f50533 mul a0,a0,a5 46: 6422 ld s0,8(sp) 48: 0185551b srliw a0,a0,0x18 "mul" instruction is leveraged as expectation, but 0x01010101 load waste several instructions. Based on this test, force to leverage "mul" instruction might be not faster than current compiler implementations. I am not sure above assembly is the best way to load 0x01010101? I checked the ISA manual, "lui" only load 20 bits per time, is this the best way to load instants? On the other hand, I try to compare ARM64 disassembly code: ..... 4: 3200c3e2 mov w2, #0x1010101 // #16843009 ...... w = (w + (w >> 4)) & 0x0f0f0f0f; 20: 0b401000 add w0, w0, w0, lsr #4 24: 1200cc00 and w0, w0, #0xf0f0f0f return (w * 0x01010101) >> 24; 28: 1b027c00 mul w0, w0, w2 Only one "mov" instructions to load 0x1010101 and one "mul" instruction for multiply. Let me summary as below: 1. GCC 10.1 cannot generate "mul" instruction when CONFIG_ARCH_HAS_FAST_MULTIPLIER enabled. 2. force to generate "mul" didn't get better because instants load waste instructions. 3. If GCC compiler behavior is best solution for this case, we could have below work around on Riscv. unsigned int __sw_hweight32(unsigned int w) { -#ifdef CONFIG_ARCH_HAS_FAST_MULTIPLIER +/* + * Risc-V could not generate mul(w) instruction in this case + */ +#if defined(CONFIG_ARCH_HAS_FAST_MULTIPLIER) && !defined(CONFIG_RISCV) w -= (w >> 1) & 0x55555555; w = (w & 0x33333333) + ((w >> 2) & 0x33333333); w = (w + (w >> 4)) & 0x0f0f0f0f; Chenxi On 2020/7/21 上午9:17, Palmer Dabbelt wrote: > On Wed, 08 Jul 2020 22:19:22 PDT (-0700), maochenxi@eswin.com wrote: >> Enable ARCH_HAS_FAST_MULTIPLIER on RV64I >> which works fine on GCC-9.3 and GCC-10.1 >> >> PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. >> >> Signed-off-by: Chenxi Mao <maochenxi@eswin.com> >> --- >> arch/riscv/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 128192e14ff2..84e6777fecad 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -202,6 +202,7 @@ config ARCH_RV64I >> bool "RV64I" >> select 64BIT >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 >> + select ARCH_HAS_FAST_MULTIPLIER >> select HAVE_DYNAMIC_FTRACE if MMU >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >> select HAVE_FTRACE_MCOUNT_RECORD > > Ah, thanks -- this one didn't show up when I was looking at the last one. I > think we can put the fast multiplier on rv32 and rv64, there shouldn't be any > difference there. I guess in theory we should be sticking this all in some > sort of "platform type" optimization flags, but that's probably bit much for > now. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I 2020-07-23 1:59 ` Chenxi Mao @ 2020-07-23 2:13 ` Palmer Dabbelt 2020-07-23 2:57 ` Chenxi Mao 0 siblings, 1 reply; 7+ messages in thread From: Palmer Dabbelt @ 2020-07-23 2:13 UTC (permalink / raw) To: maochenxi Cc: aou, kernel, wangqiang1, Paul Walmsley, linux-kernel, chenxi.mao2013, linux-riscv On Wed, 22 Jul 2020 18:59:12 PDT (-0700), maochenxi@eswin.com wrote: > Hi Palmer and Emil: > > As Emil mentioned in previous E-mail loop, I did the same test on my kernel as well. Sorry, I guess I crossed up my emails. I think it's best to just drop this for now, as it doesn't actually seem to generate better code for our current target. > > My kernel is based on Linux 5.8-RC6 with GCC-10.1. (ISA C extension enabled) > > The disassembly code as below: > > CONFIG_ARCH_HAS_FAST_MULTIPLIER enabled: > > 0000000000000000 <__sw_hweight32>: > 0: 555557b7 lui a5,0x55555 > 4: 0015571b srliw a4,a0,0x1 > 8: 55578793 addi a5,a5,1365 # 55555555 <.LASF5+0x5555509d> > c: 8ff9 and a5,a5,a4 > e: 9d1d subw a0,a0,a5 > > 0000000000000010 <.LVL1>: > 10: 333337b7 lui a5,0x33333 > 14: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332e7b> > 18: 0025571b srliw a4,a0,0x2 > 1c: 8d7d and a0,a0,a5 > 1e: 8ff9 and a5,a5,a4 > 20: 9fa9 addw a5,a5,a0 > 22: 0047d51b srliw a0,a5,0x4 > 26: 9fa9 addw a5,a5,a0 > 28: 0f0f1537 lui a0,0xf0f1 > 2c: 1141 addi sp,sp,-16 > 2e: f0f50513 addi a0,a0,-241 # f0f0f0f <.LASF5+0xf0f0a57> > 32: e422 sd s0,8(sp) > 34: 8fe9 and a5,a5,a0 > 36: 0800 addi s0,sp,16 > 38: 0087951b slliw a0,a5,0x8 > 3c: 6422 ld s0,8(sp) > 3e: 9d3d addw a0,a0,a5 > 40: 0105179b slliw a5,a0,0x10 > 44: 9d3d addw a0,a0,a5 > 46: 0185551b srliw a0,a0,0x18 > 4a: 0141 addi sp,sp,16 > 4c: 8082 ret > > CONFIG_ARCH_HAS_FAST_MULTIPLIER disabled: > > 000000000000004e <__sw_hweight32_default>: > 4e: 55555737 lui a4,0x55555 > 52: 0015579b srliw a5,a0,0x1 > 56: 55570713 addi a4,a4,1365 # 55555555 <.LASF5+0x5555509d> > 5a: 8ff9 and a5,a5,a4 > 5c: 9d1d subw a0,a0,a5 > > 000000000000005e <.LVL3>: > 5e: 333337b7 lui a5,0x33333 > 62: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332e7b> > 66: 0025571b srliw a4,a0,0x2 > 6a: 8d7d and a0,a0,a5 > 6c: 8ff9 and a5,a5,a4 > 6e: 9fa9 addw a5,a5,a0 > 70: 0047d51b srliw a0,a5,0x4 > 74: 9d3d addw a0,a0,a5 > 76: 0f0f17b7 lui a5,0xf0f1 > 7a: 1141 addi sp,sp,-16 > 7c: f0f78793 addi a5,a5,-241 # f0f0f0f <.LASF5+0xf0f0a57> > 80: e422 sd s0,8(sp) > 82: 8fe9 and a5,a5,a0 > 84: 0800 addi s0,sp,16 > 86: 0087d51b srliw a0,a5,0x8 > 8a: 6422 ld s0,8(sp) > 8c: 9fa9 addw a5,a5,a0 > 8e: 0107d51b srliw a0,a5,0x10 > 92: 9d3d addw a0,a0,a5 > 94: 0ff57513 andi a0,a0,255 > 98: 0141 addi sp,sp,16 > 9a: 8082 ret > > This 2 implementations is almost same but small differences. > > Especially in CONFIG_ARCH_HAS_FAST_MULTIPLIER condition, below code didn't use "mul" instructions. > > " return (w * 0x01010101) >> 24; " > > So I am trying to translate this code with inline assembly as below: > > //return (w * 0x01010101) >> 24; > __asm__ ( > " mul %0, %0, %1\n" > : "+r" (w) > : "r" (w), "r"(0x01010101) > :); > return w >> 24; > > After above change, the disassambly as below: > 0000000000000000 <__sw_hweight32>: > 0: 555557b7 lui a5,0x55555 > 4: 0015571b srliw a4,a0,0x1 > 8: 55578793 addi a5,a5,1365 # 55555555 <.LASF5+0x55555119> > c: 8ff9 and a5,a5,a4 > e: 9d1d subw a0,a0,a5 > > 0000000000000010 <.LVL1>: > 10: 333337b7 lui a5,0x33333 > 14: 0025571b srliw a4,a0,0x2 > 18: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332ef7> > 1c: 8d7d and a0,a0,a5 > 1e: 8ff9 and a5,a5,a4 > 20: 9fa9 addw a5,a5,a0 > 22: 0047d71b srliw a4,a5,0x4 > 26: 9f3d addw a4,a4,a5 > 28: 0f0f17b7 lui a5,0xf0f1 > 2c: 1141 addi sp,sp,-16 > 2e: f0f78793 addi a5,a5,-241 # f0f0f0f <.LASF5+0xf0f0ad3> > 32: e422 sd s0,8(sp) > 34: 8ff9 and a5,a5,a4 > 36: 0800 addi s0,sp,16 > 38: 01010737 lui a4,0x1010 > 3c: 853e mv a0,a5 > > 000000000000003e <.LVL2>: > 3e: 1017071b addiw a4,a4,257 > 42: 02f50533 mul a0,a0,a5 > 46: 6422 ld s0,8(sp) > 48: 0185551b srliw a0,a0,0x18 > > "mul" instruction is leveraged as expectation, but 0x01010101 load waste several instructions. > > Based on this test, force to leverage "mul" instruction might be not faster than current compiler implementations. > > I am not sure above assembly is the best way to load 0x01010101? I checked the ISA manual, "lui" only > > load 20 bits per time, is this the best way to load instants? > > > On the other hand, I try to compare ARM64 disassembly code: > > ..... > > 4: 3200c3e2 mov w2, #0x1010101 // #16843009 > > ...... > > w = (w + (w >> 4)) & 0x0f0f0f0f; > 20: 0b401000 add w0, w0, w0, lsr #4 > 24: 1200cc00 and w0, w0, #0xf0f0f0f > return (w * 0x01010101) >> 24; > 28: 1b027c00 mul w0, w0, w2 > > Only one "mov" instructions to load 0x1010101 and one "mul" instruction for multiply. > > > Let me summary as below: > > 1. GCC 10.1 cannot generate "mul" instruction when CONFIG_ARCH_HAS_FAST_MULTIPLIER enabled. > > 2. force to generate "mul" didn't get better because instants load waste instructions. > > 3. If GCC compiler behavior is best solution for this case, we could have below work around on Riscv. > > unsigned int __sw_hweight32(unsigned int w) > { > -#ifdef CONFIG_ARCH_HAS_FAST_MULTIPLIER > +/* > + * Risc-V could not generate mul(w) instruction in this case > + */ > +#if defined(CONFIG_ARCH_HAS_FAST_MULTIPLIER) && !defined(CONFIG_RISCV) > w -= (w >> 1) & 0x55555555; > w = (w & 0x33333333) + ((w >> 2) & 0x33333333); > w = (w + (w >> 4)) & 0x0f0f0f0f; > > > Chenxi > > > On 2020/7/21 上午9:17, Palmer Dabbelt wrote: >> On Wed, 08 Jul 2020 22:19:22 PDT (-0700), maochenxi@eswin.com wrote: >>> Enable ARCH_HAS_FAST_MULTIPLIER on RV64I >>> which works fine on GCC-9.3 and GCC-10.1 >>> >>> PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. >>> >>> Signed-off-by: Chenxi Mao <maochenxi@eswin.com> >>> --- >>> arch/riscv/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >>> index 128192e14ff2..84e6777fecad 100644 >>> --- a/arch/riscv/Kconfig >>> +++ b/arch/riscv/Kconfig >>> @@ -202,6 +202,7 @@ config ARCH_RV64I >>> bool "RV64I" >>> select 64BIT >>> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 >>> + select ARCH_HAS_FAST_MULTIPLIER >>> select HAVE_DYNAMIC_FTRACE if MMU >>> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >>> select HAVE_FTRACE_MCOUNT_RECORD >> >> Ah, thanks -- this one didn't show up when I was looking at the last one. I >> think we can put the fast multiplier on rv32 and rv64, there shouldn't be any >> difference there. I guess in theory we should be sticking this all in some >> sort of "platform type" optimization flags, but that's probably bit much for >> now. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I 2020-07-23 2:13 ` Palmer Dabbelt @ 2020-07-23 2:57 ` Chenxi Mao 0 siblings, 0 replies; 7+ messages in thread From: Chenxi Mao @ 2020-07-23 2:57 UTC (permalink / raw) To: Palmer Dabbelt Cc: aou, kernel, wangqiang1, Paul Walmsley, linux-kernel, chenxi.mao2013, linux-riscv Hi Palmer: Did you mean we drop this totally or drop this for __sw_hweight32 only? Chenxi On 2020/7/23 上午10:13, Palmer Dabbelt wrote: > On Wed, 22 Jul 2020 18:59:12 PDT (-0700), maochenxi@eswin.com wrote: >> Hi Palmer and Emil: >> >> As Emil mentioned in previous E-mail loop, I did the same test on my kernel as well. > > Sorry, I guess I crossed up my emails. I think it's best to just drop this for > now, as it doesn't actually seem to generate better code for our current > target. > >> >> My kernel is based on Linux 5.8-RC6 with GCC-10.1. (ISA C extension enabled) >> >> The disassembly code as below: >> >> CONFIG_ARCH_HAS_FAST_MULTIPLIER enabled: >> >> 0000000000000000 <__sw_hweight32>: >> 0: 555557b7 lui a5,0x55555 >> 4: 0015571b srliw a4,a0,0x1 >> 8: 55578793 addi a5,a5,1365 # 55555555 <.LASF5+0x5555509d> >> c: 8ff9 and a5,a5,a4 >> e: 9d1d subw a0,a0,a5 >> >> 0000000000000010 <.LVL1>: >> 10: 333337b7 lui a5,0x33333 >> 14: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332e7b> >> 18: 0025571b srliw a4,a0,0x2 >> 1c: 8d7d and a0,a0,a5 >> 1e: 8ff9 and a5,a5,a4 >> 20: 9fa9 addw a5,a5,a0 >> 22: 0047d51b srliw a0,a5,0x4 >> 26: 9fa9 addw a5,a5,a0 >> 28: 0f0f1537 lui a0,0xf0f1 >> 2c: 1141 addi sp,sp,-16 >> 2e: f0f50513 addi a0,a0,-241 # f0f0f0f <.LASF5+0xf0f0a57> >> 32: e422 sd s0,8(sp) >> 34: 8fe9 and a5,a5,a0 >> 36: 0800 addi s0,sp,16 >> 38: 0087951b slliw a0,a5,0x8 >> 3c: 6422 ld s0,8(sp) >> 3e: 9d3d addw a0,a0,a5 >> 40: 0105179b slliw a5,a0,0x10 >> 44: 9d3d addw a0,a0,a5 >> 46: 0185551b srliw a0,a0,0x18 >> 4a: 0141 addi sp,sp,16 >> 4c: 8082 ret >> >> CONFIG_ARCH_HAS_FAST_MULTIPLIER disabled: >> >> 000000000000004e <__sw_hweight32_default>: >> 4e: 55555737 lui a4,0x55555 >> 52: 0015579b srliw a5,a0,0x1 >> 56: 55570713 addi a4,a4,1365 # 55555555 <.LASF5+0x5555509d> >> 5a: 8ff9 and a5,a5,a4 >> 5c: 9d1d subw a0,a0,a5 >> >> 000000000000005e <.LVL3>: >> 5e: 333337b7 lui a5,0x33333 >> 62: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332e7b> >> 66: 0025571b srliw a4,a0,0x2 >> 6a: 8d7d and a0,a0,a5 >> 6c: 8ff9 and a5,a5,a4 >> 6e: 9fa9 addw a5,a5,a0 >> 70: 0047d51b srliw a0,a5,0x4 >> 74: 9d3d addw a0,a0,a5 >> 76: 0f0f17b7 lui a5,0xf0f1 >> 7a: 1141 addi sp,sp,-16 >> 7c: f0f78793 addi a5,a5,-241 # f0f0f0f <.LASF5+0xf0f0a57> >> 80: e422 sd s0,8(sp) >> 82: 8fe9 and a5,a5,a0 >> 84: 0800 addi s0,sp,16 >> 86: 0087d51b srliw a0,a5,0x8 >> 8a: 6422 ld s0,8(sp) >> 8c: 9fa9 addw a5,a5,a0 >> 8e: 0107d51b srliw a0,a5,0x10 >> 92: 9d3d addw a0,a0,a5 >> 94: 0ff57513 andi a0,a0,255 >> 98: 0141 addi sp,sp,16 >> 9a: 8082 ret >> >> This 2 implementations is almost same but small differences. >> >> Especially in CONFIG_ARCH_HAS_FAST_MULTIPLIER condition, below code didn't use "mul" instructions. >> >> " return (w * 0x01010101) >> 24; " >> >> So I am trying to translate this code with inline assembly as below: >> >> //return (w * 0x01010101) >> 24; >> __asm__ ( >> " mul %0, %0, %1\n" >> : "+r" (w) >> : "r" (w), "r"(0x01010101) >> :); >> return w >> 24; >> >> After above change, the disassambly as below: >> 0000000000000000 <__sw_hweight32>: >> 0: 555557b7 lui a5,0x55555 >> 4: 0015571b srliw a4,a0,0x1 >> 8: 55578793 addi a5,a5,1365 # 55555555 <.LASF5+0x55555119> >> c: 8ff9 and a5,a5,a4 >> e: 9d1d subw a0,a0,a5 >> >> 0000000000000010 <.LVL1>: >> 10: 333337b7 lui a5,0x33333 >> 14: 0025571b srliw a4,a0,0x2 >> 18: 33378793 addi a5,a5,819 # 33333333 <.LASF5+0x33332ef7> >> 1c: 8d7d and a0,a0,a5 >> 1e: 8ff9 and a5,a5,a4 >> 20: 9fa9 addw a5,a5,a0 >> 22: 0047d71b srliw a4,a5,0x4 >> 26: 9f3d addw a4,a4,a5 >> 28: 0f0f17b7 lui a5,0xf0f1 >> 2c: 1141 addi sp,sp,-16 >> 2e: f0f78793 addi a5,a5,-241 # f0f0f0f <.LASF5+0xf0f0ad3> >> 32: e422 sd s0,8(sp) >> 34: 8ff9 and a5,a5,a4 >> 36: 0800 addi s0,sp,16 >> 38: 01010737 lui a4,0x1010 >> 3c: 853e mv a0,a5 >> >> 000000000000003e <.LVL2>: >> 3e: 1017071b addiw a4,a4,257 >> 42: 02f50533 mul a0,a0,a5 >> 46: 6422 ld s0,8(sp) >> 48: 0185551b srliw a0,a0,0x18 >> >> "mul" instruction is leveraged as expectation, but 0x01010101 load waste several instructions. >> >> Based on this test, force to leverage "mul" instruction might be not faster than current compiler implementations. >> >> I am not sure above assembly is the best way to load 0x01010101? I checked the ISA manual, "lui" only >> >> load 20 bits per time, is this the best way to load instants? >> >> >> On the other hand, I try to compare ARM64 disassembly code: >> >> ..... >> >> 4: 3200c3e2 mov w2, #0x1010101 // #16843009 >> >> ...... >> >> w = (w + (w >> 4)) & 0x0f0f0f0f; >> 20: 0b401000 add w0, w0, w0, lsr #4 >> 24: 1200cc00 and w0, w0, #0xf0f0f0f >> return (w * 0x01010101) >> 24; >> 28: 1b027c00 mul w0, w0, w2 >> >> Only one "mov" instructions to load 0x1010101 and one "mul" instruction for multiply. >> >> >> Let me summary as below: >> >> 1. GCC 10.1 cannot generate "mul" instruction when CONFIG_ARCH_HAS_FAST_MULTIPLIER enabled. >> >> 2. force to generate "mul" didn't get better because instants load waste instructions. >> >> 3. If GCC compiler behavior is best solution for this case, we could have below work around on Riscv. >> >> unsigned int __sw_hweight32(unsigned int w) >> { >> -#ifdef CONFIG_ARCH_HAS_FAST_MULTIPLIER >> +/* >> + * Risc-V could not generate mul(w) instruction in this case >> + */ >> +#if defined(CONFIG_ARCH_HAS_FAST_MULTIPLIER) && !defined(CONFIG_RISCV) >> w -= (w >> 1) & 0x55555555; >> w = (w & 0x33333333) + ((w >> 2) & 0x33333333); >> w = (w + (w >> 4)) & 0x0f0f0f0f; >> >> >> Chenxi >> >> >> On 2020/7/21 上午9:17, Palmer Dabbelt wrote: >>> On Wed, 08 Jul 2020 22:19:22 PDT (-0700), maochenxi@eswin.com wrote: >>>> Enable ARCH_HAS_FAST_MULTIPLIER on RV64I >>>> which works fine on GCC-9.3 and GCC-10.1 >>>> >>>> PS2: remove ARCH_SUPPORTS_INT128 because of RV64I already enabled. >>>> >>>> Signed-off-by: Chenxi Mao <maochenxi@eswin.com> >>>> --- >>>> arch/riscv/Kconfig | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >>>> index 128192e14ff2..84e6777fecad 100644 >>>> --- a/arch/riscv/Kconfig >>>> +++ b/arch/riscv/Kconfig >>>> @@ -202,6 +202,7 @@ config ARCH_RV64I >>>> bool "RV64I" >>>> select 64BIT >>>> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000 >>>> + select ARCH_HAS_FAST_MULTIPLIER >>>> select HAVE_DYNAMIC_FTRACE if MMU >>>> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE >>>> select HAVE_FTRACE_MCOUNT_RECORD >>> >>> Ah, thanks -- this one didn't show up when I was looking at the last one. I >>> think we can put the fast multiplier on rv32 and rv64, there shouldn't be any >>> difference there. I guess in theory we should be sticking this all in some >>> sort of "platform type" optimization flags, but that's probably bit much for >>> now. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-23 2:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-09 5:19 [PATCH 1/1] riscv: Enable ARCH_HAS_FAST_MULTIPLIER for RV64I Chenxi Mao 2020-07-21 1:17 ` Palmer Dabbelt 2020-07-21 1:47 ` Chenxi Mao 2020-07-21 2:38 ` Chenxi Mao 2020-07-23 1:59 ` Chenxi Mao 2020-07-23 2:13 ` Palmer Dabbelt 2020-07-23 2:57 ` Chenxi Mao
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 NNTP newsgroup(s).