[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <E0E58901-5DCF-46A2-9B96-8FFEF9C0EB64@jrtc27.com>
Date: Wed, 8 Jan 2025 21:09:46 +0000
From: Jessica Clarke <jrtc27@...c27.com>
To: Conor Dooley <conor@...nel.org>
Cc: Chunyan Zhang <zhang.lyra@...il.com>,
Chunyan Zhang <zhangchunyan@...as.ac.cn>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Song Liu <song@...nel.org>,
Yu Kuai <yukuai3@...wei.com>,
linux-riscv@...ts.infradead.org,
linux-raid@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] raid6: Add RISC-V SIMD syndrome and recovery
calculations
On 8 Jan 2025, at 18:57, Conor Dooley <conor@...nel.org> wrote:
>
> On Mon, Dec 23, 2024 at 10:16:46AM +0800, Chunyan Zhang wrote:
>> On Mon, 23 Dec 2024 at 09:35, Conor Dooley <conor@...nel.org> wrote:
>>>
>>> On Mon, Dec 23, 2024 at 09:16:38AM +0800, Chunyan Zhang wrote:
>>>> Hi Conor,
>>>>
>>>> On Sat, 21 Dec 2024 at 06:52, Conor Dooley <conor@...nel.org> wrote:
>>>>>
>>>>> On Fri, Dec 20, 2024 at 07:40:23PM +0800, Chunyan Zhang wrote:
>>>>>> The assembly is originally based on the ARM NEON and int.uc, but uses
>>>>>> RISC-V vector instructions to implement the RAID6 syndrome and
>>>>>> recovery calculations.
>>>>>>
>>>>>> The functions are tested on QEMU.
>>>>>>
>>>>>> Signed-off-by: Chunyan Zhang <zhangchunyan@...as.ac.cn>
>>>>>> ---
>>>>>> include/linux/raid/pq.h | 4 +
>>>>>> lib/raid6/Makefile | 3 +
>>>>>> lib/raid6/algos.c | 8 +
>>>>>> lib/raid6/recov_rvv.c | 229 +++++++++++++
>>>>>> lib/raid6/rvv.c | 715 ++++++++++++++++++++++++++++++++++++++++
>>>>>> 5 files changed, 959 insertions(+)
>>>>>> create mode 100644 lib/raid6/recov_rvv.c
>>>>>> create mode 100644 lib/raid6/rvv.c
>>>>>>
>>>>>> diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h
>>>>>> index 98030accf641..4c21f06c662a 100644
>>>>>> --- a/include/linux/raid/pq.h
>>>>>> +++ b/include/linux/raid/pq.h
>>>>>> @@ -108,6 +108,9 @@ extern const struct raid6_calls raid6_vpermxor4;
>>>>>> extern const struct raid6_calls raid6_vpermxor8;
>>>>>> extern const struct raid6_calls raid6_lsx;
>>>>>> extern const struct raid6_calls raid6_lasx;
>>>>>> +extern const struct raid6_calls raid6_rvvx1;
>>>>>> +extern const struct raid6_calls raid6_rvvx2;
>>>>>> +extern const struct raid6_calls raid6_rvvx4;
>>>>>>
>>>>>> struct raid6_recov_calls {
>>>>>> void (*data2)(int, size_t, int, int, void **);
>>>>>> @@ -125,6 +128,7 @@ extern const struct raid6_recov_calls raid6_recov_s390xc;
>>>>>> extern const struct raid6_recov_calls raid6_recov_neon;
>>>>>> extern const struct raid6_recov_calls raid6_recov_lsx;
>>>>>> extern const struct raid6_recov_calls raid6_recov_lasx;
>>>>>> +extern const struct raid6_recov_calls raid6_recov_rvv;
>>>>>>
>>>>>> extern const struct raid6_calls raid6_neonx1;
>>>>>> extern const struct raid6_calls raid6_neonx2;
>>>>>> diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
>>>>>> index 29127dd05d63..e62fb7cd773e 100644
>>>>>> --- a/lib/raid6/Makefile
>>>>>> +++ b/lib/raid6/Makefile
>>>>>> @@ -10,6 +10,9 @@ raid6_pq-$(CONFIG_ALTIVEC) += altivec1.o altivec2.o altivec4.o altivec8.o \
>>>>>> raid6_pq-$(CONFIG_KERNEL_MODE_NEON) += neon.o neon1.o neon2.o neon4.o neon8.o recov_neon.o recov_neon_inner.o
>>>>>> raid6_pq-$(CONFIG_S390) += s390vx8.o recov_s390xc.o
>>>>>> raid6_pq-$(CONFIG_LOONGARCH) += loongarch_simd.o recov_loongarch_simd.o
>>>>>> +raid6_pq-$(CONFIG_RISCV_ISA_V) += rvv.o recov_rvv.o
>>>>>> +CFLAGS_rvv.o += -march=rv64gcv
>>>>>> +CFLAGS_recov_rvv.o += -march=rv64gcv
>>>>>
>>>>> I'm curious - why do you need this when you're using .option arch,+v
>>>>> below?
>>>>
>>>> Compiler would complain the errors like below without this flag:
>>>>
>>>> Error: unrecognized opcode `vle8.v v0,(a3)', extension `v' or `zve64x'
>>>> or `zve32x' required
>>>
>>> Right, but the reason for using .option arch,+v elsewhere in the kernel
>>> is because we don't want the compiler to generate vector code at all,
>>> and the directive lets the assembler handle the vector instructions. If
>>> I recall correctly, the error you pasted above is from the assembler,
>>
>> Yes, it is from the assembler.
>>
>>> not the compiler. You should be able to just set AFLAGS, given that all
>>
>> It complains the same errors after simply replacing CFLAGS with AFLAGS
>> here. What am I missing?
>>
>
> I don't know what you're missing unfortunately, sorry.
.option push should be paired with .option pop within the same inline
asm statement. You cannot generally split them up, as the compiler will
not guarantee that the inline asm statements appear in that order
textually, since it may reorder blocks, and may even clone or delete
them. Plus in some cases you could end up affecting the compiler’s own
generated code, although that shouldn’t matter here.
Jess
Powered by blists - more mailing lists