Messages in this thread | | | Date | Thu, 18 Jan 2024 14:38:46 -0800 | From | Charlie Jenkins <> | Subject | Re: [PATCH] riscv: lib: Support csum on GCC <11 |
| |
On Thu, Jan 18, 2024 at 02:20:42PM -0800, Palmer Dabbelt wrote: > 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
I sent an updated version that uses CC_HAS_ASM_GOTO_TIED_OUTPUT since there may exist a compiler that supports ZBB but doesn't support output in asm goto.
- Charlie
> > 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 > > > > > >
| |