| Message ID | 20240118-csum_remove_output_operands_asm_goto-v2-1-5d1b73cf93d4@rivosinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4525462dd0db9e86bb67c10dedbbaa4f8d62697d |
| Headers | show |
| Series | [v2] riscv: lib: Check if output in asm goto supported | expand |
On Thu, Jan 18, 2024 at 02:36:45PM -0800, Charlie Jenkins wrote: > The output field of an asm goto statement is not supported by all > compilers. If it is not supported, fallback to the non-optimized code. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > Fixes: a04c192eabfb ("riscv: Add checksum library") > --- > The OutputOperands field for asm goto statements is only supported > starting from GCC 11. Split the asm goto to remove the use of this > feature. > --- > Changes in v2: > - Use CC_HAS_ASM_GOTO_TIED_OUTPUT > - Link to v1: https://lore.kernel.org/r/20240118-csum_remove_output_operands_asm_goto-v1-1-47c672bb9d4b@rivosinc.com > --- > arch/riscv/lib/csum.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c > index 06ce8e7250d9..af3df5274ccb 100644 > --- a/arch/riscv/lib/csum.c > +++ b/arch/riscv/lib/csum.c > @@ -156,6 +156,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) > end = (const unsigned long *)(buff + len); > csum = do_csum_common(ptr, end, data); > > +#ifdef CC_HAS_ASM_GOTO_TIED_OUTPUT Can't we just add another IS_ENABLED() to the if rather than this #ifdef? > /* > * Zbb support saves 6 instructions, so not worth checking without > * alternatives if supported > @@ -214,6 +215,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) > return csum >> 16; > } > no_zbb: > +#endif /* CC_HAS_ASM_GOTO_TIED_OUTPUT */ > #ifndef CONFIG_32BIT > csum += ror64(csum, 32); > csum >>= 32; BTW, I wonder how/if the check for CC_HAS_ASM_GOTO_TIED_OUTPUT in init/Kconfig is working as expected. I see $CC, as opposed to $(CC), being used there. I believe $CC is just the expansion of $C with a 'C' appended. Thanks, drew > > --- > base-commit: 080c4324fa5e81ff3780206a138223abfb57a68e > change-id: 20240118-csum_remove_output_operands_asm_goto-49922c141ce7 > -- > - Charlie > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, Jan 19, 2024 at 10:41:14AM +0100, Andrew Jones wrote: > On Thu, Jan 18, 2024 at 02:36:45PM -0800, Charlie Jenkins wrote: > > The output field of an asm goto statement is not supported by all > > compilers. If it is not supported, fallback to the non-optimized code. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > Fixes: a04c192eabfb ("riscv: Add checksum library") > > --- > > The OutputOperands field for asm goto statements is only supported > > starting from GCC 11. Split the asm goto to remove the use of this > > feature. > > --- > > Changes in v2: > > - Use CC_HAS_ASM_GOTO_TIED_OUTPUT > > - Link to v1: https://lore.kernel.org/r/20240118-csum_remove_output_operands_asm_goto-v1-1-47c672bb9d4b@rivosinc.com > > --- > > arch/riscv/lib/csum.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c > > index 06ce8e7250d9..af3df5274ccb 100644 > > --- a/arch/riscv/lib/csum.c > > +++ b/arch/riscv/lib/csum.c > > @@ -156,6 +156,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) > > end = (const unsigned long *)(buff + len); > > csum = do_csum_common(ptr, end, data); > > > > +#ifdef CC_HAS_ASM_GOTO_TIED_OUTPUT > > Can't we just add another IS_ENABLED() to the if rather than this #ifdef? Unfortunately no. GCC throws syntax before it determines if a branch will never be taken, so even though the code is not emitted it will still fail with IS_ENABLED. > > > /* > > * Zbb support saves 6 instructions, so not worth checking without > > * alternatives if supported > > @@ -214,6 +215,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) > > return csum >> 16; > > } > > no_zbb: > > +#endif /* CC_HAS_ASM_GOTO_TIED_OUTPUT */ > > #ifndef CONFIG_32BIT > > csum += ror64(csum, 32); > > csum >>= 32; > > BTW, I wonder how/if the check for CC_HAS_ASM_GOTO_TIED_OUTPUT in > init/Kconfig is working as expected. I see $CC, as opposed to $(CC), > being used there. I believe $CC is just the expansion of $C with a > 'C' appended. Huh that is strange. It does work but I am not sure how. - Charlie > > Thanks, > drew > > > > > --- > > base-commit: 080c4324fa5e81ff3780206a138223abfb57a68e > > change-id: 20240118-csum_remove_output_operands_asm_goto-49922c141ce7 > > -- > > - Charlie > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hello: This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt <palmer@rivosinc.com>: On Thu, 18 Jan 2024 14:36:45 -0800 you wrote: > The output field of an asm goto statement is not supported by all > compilers. If it is not supported, fallback to the non-optimized code. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > Fixes: a04c192eabfb ("riscv: Add checksum library") > --- > The OutputOperands field for asm goto statements is only supported > starting from GCC 11. Split the asm goto to remove the use of this > feature. > > [...] Here is the summary with links: - [v2] riscv: lib: Check if output in asm goto supported https://git.kernel.org/riscv/c/4525462dd0db You are awesome, thank you!
diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c index 06ce8e7250d9..af3df5274ccb 100644 --- a/arch/riscv/lib/csum.c +++ b/arch/riscv/lib/csum.c @@ -156,6 +156,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) end = (const unsigned long *)(buff + len); csum = do_csum_common(ptr, end, data); +#ifdef CC_HAS_ASM_GOTO_TIED_OUTPUT /* * Zbb support saves 6 instructions, so not worth checking without * alternatives if supported @@ -214,6 +215,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) return csum >> 16; } no_zbb: +#endif /* CC_HAS_ASM_GOTO_TIED_OUTPUT */ #ifndef CONFIG_32BIT csum += ror64(csum, 32); csum >>= 32;
The output field of an asm goto statement is not supported by all compilers. If it is not supported, fallback to the non-optimized code. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> Fixes: a04c192eabfb ("riscv: Add checksum library") --- The OutputOperands field for asm goto statements is only supported starting from GCC 11. Split the asm goto to remove the use of this feature. --- Changes in v2: - Use CC_HAS_ASM_GOTO_TIED_OUTPUT - Link to v1: https://lore.kernel.org/r/20240118-csum_remove_output_operands_asm_goto-v1-1-47c672bb9d4b@rivosinc.com --- arch/riscv/lib/csum.c | 2 ++ 1 file changed, 2 insertions(+) --- base-commit: 080c4324fa5e81ff3780206a138223abfb57a68e change-id: 20240118-csum_remove_output_operands_asm_goto-49922c141ce7