Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 5 May 2023 13:54:56 +0200
From:   Andrew Jones <ajones@ventanamicro.com>
To:     =?utf-8?B?5byg6aOe?= <zhangfei@nj.iscas.ac.cn>
Cc:     paul.walmsley@sifive.com, palmer@dabbelt.com,
        aou@eecs.berkeley.edu, linux-riscv@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: Optimize memset
Message-ID: <20230505-9ec599a36801972451e8b17f@orel>
References: <6d1cbe2e.3c31d.187eb14d990.Coremail.zhangfei@nj.iscas.ac.cn>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <6d1cbe2e.3c31d.187eb14d990.Coremail.zhangfei@nj.iscas.ac.cn>
Precedence: bulk


Please don't post patches as attachments.

> From: zhangfei <zhangfei@nj.iscas.ac.cn>
> Date: Fri, 5 May 2023 14:58:35 +0800
> Subject: [PATCH] riscv: Optimize memset
> 
> This patch has been optimized for memset data sizes less than 16 bytes.
> Compared to byte by byte storage, significant performance improvement has been achieved.
> 
> Signed-off-by: Fei Zhang <zhangfei@nj.iscas.ac.cn>
> ---
>  arch/riscv/lib/memset.S | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index 34c5360c6705..0967bdf86bd6 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -105,9 +105,36 @@ WEAK(memset)
>         beqz a2, 6f
>         add a3, t0, a2
>  5:
> -	sb a1, 0(t0)
> -	addi t0, t0, 1
> -	bltu t0, a3, 5b
> +        sb a1, 0(t0)
> +        sb a1, -1(a3)
> +        li a4, 2
> +        bgeu a4, a2, 6f
> +
> +        sb a1, 1(t0)
> +        sb a1, 2(t0)
> +        sb a1, -2(a3)
> +        sb a1, -3(a3)
> +        li a4, 6
> +        bgeu a4, a2, 6f
> +
> +        sb a1, 3(t0)
> +        sb a1, -4(a3)
> +        li a4, 8
> +        bgeu a4, a2, 6f

Why is this check here?

> +
> +        sb a1, 4(t0)
> +        sb a1, -5(a3)
> +        li a4, 10
> +        bgeu a4, a2, 6f

And this one?

After the check of a2 against 6 above we know that offsets 6(t0)
and -7(a3) are safe. Are we trying to avoid too may redundant
stores with these additional checks?

> +
> +        sb a1, 5(t0)
> +        sb a1, 6(t0)
> +        sb a1, -6(a3)
> +        sb a1, -7(a3)
> +        li a4, 14
> +        bgeu a4, a2, 6f
> +
> +        sb a1, 7(t0)
>  6:
>    	ret
>  END(__memset)
> --
> 2.33.0

The indent of the new code doesn't match the old. I'd prefer we cleanup
the old first, though. Please repost [1] as a first patch of a two-patch
patch series, where yours is the second and matches the new formatting
that [1] uses.

[1] https://lore.kernel.org/all/20221027130247.31634-8-ajones@ventanamicro.com/

Thanks,
Drew

On Fri, May 05, 2023 at 04:43:44PM +0800, 张飞 wrote:
> 


> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv