Messages in this thread Patch in this message | | | Date | Thu, 18 Jan 2024 14:20:42 -0800 (PST) | Subject | Re: [PATCH] riscv: lib: Support csum on GCC <11 | From | Palmer Dabbelt <> |
| |
On Thu, 18 Jan 2024 14:05:44 PST (-0800), Conor Dooley wrote: > On Thu, Jan 18, 2024 at 01:53:59PM -0800, Charlie Jenkins wrote: >> 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. >> >> 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. > > Maybe this is a super naive question, but is it possible to just not > use the custom csum code for gcc older than 11?
Charlie and I were talking, these old GCC versions also don't support ZBB. So I think we can get away with something like
diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c index 06ce8e7250d9..17f883b612c9 100644 --- a/arch/riscv/lib/csum.c +++ b/arch/riscv/lib/csum.c @@ -158,10 +158,16 @@ do_csum_with_alignment(const unsigned char *buff, int len) /* * Zbb support saves 6 instructions, so not worth checking without - * alternatives if supported + * alternatives if supported. + * + * Note that we pull the check for ZBB into the preprocessor proper + * here, as otherwise GCC will attempt to compile the code inside the + * "if (IS_ENABLED(ZBB)" block which fails because GCC10 doesn't + * support ASM goto output operands. GCC 10 also doesn't support ZBB, + * so we're safe with that check here. */ - if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && - IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { +#if defined(CONFIG_RISCV_ISA_ZBB) + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { unsigned long fold_temp; /* @@ -213,6 +219,7 @@ do_csum_with_alignment(const unsigned char *buff, int len) end: return csum >> 16; } +#endif /*CONFIG_RISCV_ISA_ZBB*/ no_zbb: #ifndef CONFIG_32BIT csum += ror64(csum, 32); @@ -244,10 +251,11 @@ do_csum_no_alignment(const unsigned char *buff, int len) /* * Zbb support saves 6 instructions, so not worth checking without - * alternatives if supported + * alternatives if supported. See above for the ZBB preprocessor + * check. */ - if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && - IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { +#if defined(CONFIG_RISCV_ISA_ZBB) + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { unsigned long fold_temp; /* @@ -287,6 +295,7 @@ do_csum_no_alignment(const unsigned char *buff, int len) #endif /* !CONFIG_32BIT */ return csum >> 16; } +#endif /*CONFIG_RISCV_ISA_ZBB*/ no_zbb: #ifndef CONFIG_32BIT csum += ror64(csum, 32); which is building for me on GCC-10/defconfig.
> >> --- >> arch/riscv/lib/csum.c | 42 ++++++++++++++++++++++++++++++------------ >> 1 file changed, 30 insertions(+), 12 deletions(-) >> >> diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c >> index 06ce8e7250d9..23be289f52b6 100644 >> --- a/arch/riscv/lib/csum.c >> +++ b/arch/riscv/lib/csum.c >> @@ -177,22 +177,35 @@ do_csum_with_alignment(const unsigned char *buff, int len) >> : no_zbb); >> >> #ifdef CONFIG_32BIT >> - asm_volatile_goto(".option push \n\ >> + /* >> + * OutputOperands in asm goto is not supported until GCC 11, so >> + * this asm has to be split to be compatible. >> + */ >> + asm (".option push \n\ >> .option arch,+zbb \n\ >> rori %[fold_temp], %[csum], 16 \n\ >> andi %[offset], %[offset], 1 \n\ >> add %[csum], %[fold_temp], %[csum] \n\ >> - beq %[offset], zero, %l[end] \n\ >> - rev8 %[csum], %[csum] \n\ >> .option pop" >> : [csum] "+r" (csum), [fold_temp] "=&r" (fold_temp) >> - : [offset] "r" (offset) >> - : >> - : end); >> + : [offset] "r" (offset)); >> + >> + if (offset == 0) >> + goto end; >> + >> + asm (".option push \n\ >> + .option arch, +zbb \n\ >> + rev8 %[csum], %[csum] \n\ >> + .option pop" >> + : [csum] "+r" (csum)); >> >> return (unsigned short)csum; >> #else /* !CONFIG_32BIT */ >> - asm_volatile_goto(".option push \n\ >> + /* >> + * OutputOperands in asm goto is not supported until GCC 11, so >> + * this asm has to be split to be compatible. >> + */ >> + asm (".option push \n\ >> .option arch,+zbb \n\ >> rori %[fold_temp], %[csum], 32 \n\ >> add %[csum], %[fold_temp], %[csum] \n\ >> @@ -200,13 +213,18 @@ do_csum_with_alignment(const unsigned char *buff, int len) >> roriw %[fold_temp], %[csum], 16 \n\ >> addw %[csum], %[fold_temp], %[csum] \n\ >> andi %[offset], %[offset], 1 \n\ >> - beq %[offset], zero, %l[end] \n\ >> - rev8 %[csum], %[csum] \n\ >> .option pop" >> : [csum] "+r" (csum), [fold_temp] "=&r" (fold_temp) >> - : [offset] "r" (offset) >> - : >> - : end); >> + : [offset] "r" (offset)); >> + >> + if (offset == 0) >> + goto end; >> + >> + asm (".option push \n\ >> + .option arch, +zbb \n\ >> + rev8 %[csum], %[csum] \n\ >> + .option pop" >> + : [csum] "+r" (csum)); >> >> return (csum << 16) >> 48; >> #endif /* !CONFIG_32BIT */ >> >> --- >> base-commit: 080c4324fa5e81ff3780206a138223abfb57a68e >> change-id: 20240118-csum_remove_output_operands_asm_goto-49922c141ce7 >> -- >> - Charlie >> >>
| |