[8/9] RISC-V: lib: Use named labels in memset

Message ID	20221027130247.31634-9-ajones@ventanamicro.com (mailing list archive)
State	Changes Requested
Delegated to:	Palmer Dabbelt
Headers	show Return-Path: <linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org> From: Andrew Jones <ajones@ventanamicro.com> To: linux-riscv@lists.infradead.org, kvm-riscv@lists.infradead.org Cc: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Anup Patel <apatel@ventanamicro.com>, Heiko Stuebner <heiko@sntech.de>, Conor Dooley <conor.dooley@microchip.com>, Atish Patra <atishp@rivosinc.com>, Jisheng Zhang <jszhang@kernel.org> Subject: [PATCH 8/9] RISC-V: lib: Use named labels in memset Date: Thu, 27 Oct 2022 15:02:46 +0200 Message-Id: <20221027130247.31634-9-ajones@ventanamicro.com> In-Reply-To: <20221027130247.31634-1-ajones@ventanamicro.com> References: <20221027130247.31634-1-ajones@ventanamicro.com> MIME-Version: 1.0 Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org> Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org
Series	RISC-V: Apply Zicboz to clear_page and memset \| expand [0/9] RISC-V: Apply Zicboz to clear_page and memset [1/9] RISC-V: Factor out body of riscv_init_cbom_blocksize loop [2/9] RISC-V: Add Zicboz detection and block size parsing [3/9] RISC-V: insn-def: Define cbo.zero [4/9] RISC-V: Use Zicboz in clear_page when available [5/9] RISC-V: KVM: Provide UAPI for Zicboz block size [6/9] RISC-V: KVM: Expose Zicboz to the guest [7/9] RISC-V: lib: Improve memset assembler formatting [8/9] RISC-V: lib: Use named labels in memset [9/9] RISC-V: Use Zicboz in memset when available

Message ID

20221027130247.31634-9-ajones@ventanamicro.com (mailing list archive)

State

Changes Requested

Delegated to:

Palmer Dabbelt

Headers

From: Andrew Jones <ajones@ventanamicro.com>
To: linux-riscv@lists.infradead.org,
	kvm-riscv@lists.infradead.org
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Anup Patel <apatel@ventanamicro.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Conor Dooley <conor.dooley@microchip.com>,
	Atish Patra <atishp@rivosinc.com>,
	Jisheng Zhang <jszhang@kernel.org>
Subject: [PATCH 8/9] RISC-V: lib: Use named labels in memset
Date: Thu, 27 Oct 2022 15:02:46 +0200
Message-Id: <20221027130247.31634-9-ajones@ventanamicro.com>
In-Reply-To: <20221027130247.31634-1-ajones@ventanamicro.com>
References: <20221027130247.31634-1-ajones@ventanamicro.com>
MIME-Version: 1.0
Precedence: list
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org>
Errors-To: 
 linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org

Series

RISC-V: Apply Zicboz to clear_page and memset | expand

Commit Message

Andrew Jones Oct. 27, 2022, 1:02 p.m. UTC

In a coming patch we'll be adding more branch targets. Let's
change the numeric labels to named labels to make it easier
to read and integrate with.

No functional change intended.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

Conor Dooley Oct. 30, 2022, 10:15 p.m. UTC | #1

On Thu, Oct 27, 2022 at 03:02:46PM +0200, Andrew Jones wrote:
> In a coming patch we'll be adding more branch targets. Let's

rather minor nit: I don't think "In a coming patch" will be
particularly meaningful if anyone does some history diving.

> change the numeric labels to named labels to make it easier
> to read and integrate with.
> 
> No functional change intended.

It looks fine to me..
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> index e613c5c27998..74e4c7feec00 100644
> --- a/arch/riscv/lib/memset.S
> +++ b/arch/riscv/lib/memset.S
> @@ -13,7 +13,7 @@ WEAK(memset)
>  
>  	/* Defer to byte-oriented fill for small sizes */
>  	sltiu	a3, a2, 16
> -	bnez	a3, 4f
> +	bnez	a3, .Lfinish
>  
>  	/*
>  	 * Round to nearest XLEN-aligned address
> @@ -21,17 +21,18 @@ WEAK(memset)
>  	 */
>  	addi	a3, t0, SZREG-1
>  	andi	a3, a3, ~(SZREG-1)
> -	beq	a3, t0, 2f		/* Skip if already aligned */
> +	beq	a3, t0, .Ldo_duff	/* Skip if already aligned */
>  
>  	/* Handle initial misalignment */
>  	sub	a4, a3, t0
> -1:
> +.Lmisaligned1:
>  	sb	a1, 0(t0)
>  	addi	t0, t0, 1
> -	bltu	t0, a3, 1b
> +	bltu	t0, a3, .Lmisaligned1
>  	sub	a2, a2, a4		/* Update count */
>  
> -2: /* Duff's device with 32 XLEN stores per iteration */
> +.Ldo_duff:
> +	/* Duff's device with 32 XLEN stores per iteration */
>  	/* Broadcast value into all bytes */
>  	andi	a1, a1, 0xff
>  	slli	a3, a1, 8
> @@ -48,7 +49,7 @@ WEAK(memset)
>  	add	a3, t0, a4
>  
>  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
> -	beqz	a4, 3f			/* Shortcut if no remainder */
> +	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
>  	neg	a4, a4
>  	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
>  
> @@ -57,13 +58,13 @@ WEAK(memset)
>  
>  	/* Jump into loop body */
>  	/* Assumes 32-bit instruction lengths */
> -	la	a5, 3f
> +	la	a5, .Lduff_loop
>  #ifdef CONFIG_64BIT
>  	srli	a4, a4, 1
>  #endif
>  	add	a5, a5, a4
>  	jr	a5
> -3:
> +.Lduff_loop:
>  	REG_S	a1,        0(t0)
>  	REG_S	a1,    SZREG(t0)
>  	REG_S	a1,  2*SZREG(t0)
> @@ -98,17 +99,17 @@ WEAK(memset)
>  	REG_S	a1, 31*SZREG(t0)
>  
>  	addi	t0, t0, 32*SZREG
> -	bltu	t0, a3, 3b
> +	bltu	t0, a3, .Lduff_loop
>  	andi	a2, a2, SZREG-1		/* Update count */
>  
> -4:
> +.Lfinish:
>  	/* Handle trailing misalignment */
> -	beqz	a2, 6f
> +	beqz	a2, .Ldone
>  	add	a3, t0, a2
> -5:
> +.Lmisaligned2:
>  	sb	a1, 0(t0)
>  	addi	t0, t0, 1
> -	bltu	t0, a3, 5b
> -6:
> +	bltu	t0, a3, .Lmisaligned2
> +.Ldone:
>  	ret
>  END(__memset)
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Andrew Jones Oct. 31, 2022, 8:24 a.m. UTC | #2

On Sun, Oct 30, 2022 at 10:15:43PM +0000, Conor Dooley wrote:
> On Thu, Oct 27, 2022 at 03:02:46PM +0200, Andrew Jones wrote:
> > In a coming patch we'll be adding more branch targets. Let's
> 
> rather minor nit: I don't think "In a coming patch" will be
> particularly meaningful if anyone does some history diving.

Yes, I struggle with these. I think "next patch" is typically frowned upon
because things get reordered and then 'next' is no longer appropriate, but
it is good to convey somehow that while this patch doesn't provide much
use on its own it does serve a preparation purpose. Maybe I should just
write "Improve readability and prepare for extensions which will add
more labels..."

I'll do something like that for v2.

> 
> > change the numeric labels to named labels to make it easier
> > to read and integrate with.
> > 
> > No functional change intended.
> 
> It looks fine to me..
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
drew

> 
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/lib/memset.S | 29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
> > index e613c5c27998..74e4c7feec00 100644
> > --- a/arch/riscv/lib/memset.S
> > +++ b/arch/riscv/lib/memset.S
> > @@ -13,7 +13,7 @@ WEAK(memset)
> >  
> >  	/* Defer to byte-oriented fill for small sizes */
> >  	sltiu	a3, a2, 16
> > -	bnez	a3, 4f
> > +	bnez	a3, .Lfinish
> >  
> >  	/*
> >  	 * Round to nearest XLEN-aligned address
> > @@ -21,17 +21,18 @@ WEAK(memset)
> >  	 */
> >  	addi	a3, t0, SZREG-1
> >  	andi	a3, a3, ~(SZREG-1)
> > -	beq	a3, t0, 2f		/* Skip if already aligned */
> > +	beq	a3, t0, .Ldo_duff	/* Skip if already aligned */
> >  
> >  	/* Handle initial misalignment */
> >  	sub	a4, a3, t0
> > -1:
> > +.Lmisaligned1:
> >  	sb	a1, 0(t0)
> >  	addi	t0, t0, 1
> > -	bltu	t0, a3, 1b
> > +	bltu	t0, a3, .Lmisaligned1
> >  	sub	a2, a2, a4		/* Update count */
> >  
> > -2: /* Duff's device with 32 XLEN stores per iteration */
> > +.Ldo_duff:
> > +	/* Duff's device with 32 XLEN stores per iteration */
> >  	/* Broadcast value into all bytes */
> >  	andi	a1, a1, 0xff
> >  	slli	a3, a1, 8
> > @@ -48,7 +49,7 @@ WEAK(memset)
> >  	add	a3, t0, a4
> >  
> >  	andi	a4, a4, 31*SZREG	/* Calculate remainder */
> > -	beqz	a4, 3f			/* Shortcut if no remainder */
> > +	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
> >  	neg	a4, a4
> >  	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
> >  
> > @@ -57,13 +58,13 @@ WEAK(memset)
> >  
> >  	/* Jump into loop body */
> >  	/* Assumes 32-bit instruction lengths */
> > -	la	a5, 3f
> > +	la	a5, .Lduff_loop
> >  #ifdef CONFIG_64BIT
> >  	srli	a4, a4, 1
> >  #endif
> >  	add	a5, a5, a4
> >  	jr	a5
> > -3:
> > +.Lduff_loop:
> >  	REG_S	a1,        0(t0)
> >  	REG_S	a1,    SZREG(t0)
> >  	REG_S	a1,  2*SZREG(t0)
> > @@ -98,17 +99,17 @@ WEAK(memset)
> >  	REG_S	a1, 31*SZREG(t0)
> >  
> >  	addi	t0, t0, 32*SZREG
> > -	bltu	t0, a3, 3b
> > +	bltu	t0, a3, .Lduff_loop
> >  	andi	a2, a2, SZREG-1		/* Update count */
> >  
> > -4:
> > +.Lfinish:
> >  	/* Handle trailing misalignment */
> > -	beqz	a2, 6f
> > +	beqz	a2, .Ldone
> >  	add	a3, t0, a2
> > -5:
> > +.Lmisaligned2:
> >  	sb	a1, 0(t0)
> >  	addi	t0, t0, 1
> > -	bltu	t0, a3, 5b
> > -6:
> > +	bltu	t0, a3, .Lmisaligned2
> > +.Ldone:
> >  	ret
> >  END(__memset)
> > -- 
> > 2.37.3
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
index e613c5c27998..74e4c7feec00 100644
--- a/arch/riscv/lib/memset.S
+++ b/arch/riscv/lib/memset.S
@@ -13,7 +13,7 @@  WEAK(memset)
 
 	/* Defer to byte-oriented fill for small sizes */
 	sltiu	a3, a2, 16
-	bnez	a3, 4f
+	bnez	a3, .Lfinish
 
 	/*
 	 * Round to nearest XLEN-aligned address
@@ -21,17 +21,18 @@  WEAK(memset)
 	 */
 	addi	a3, t0, SZREG-1
 	andi	a3, a3, ~(SZREG-1)
-	beq	a3, t0, 2f		/* Skip if already aligned */
+	beq	a3, t0, .Ldo_duff	/* Skip if already aligned */
 
 	/* Handle initial misalignment */
 	sub	a4, a3, t0
-1:
+.Lmisaligned1:
 	sb	a1, 0(t0)
 	addi	t0, t0, 1
-	bltu	t0, a3, 1b
+	bltu	t0, a3, .Lmisaligned1
 	sub	a2, a2, a4		/* Update count */
 
-2: /* Duff's device with 32 XLEN stores per iteration */
+.Ldo_duff:
+	/* Duff's device with 32 XLEN stores per iteration */
 	/* Broadcast value into all bytes */
 	andi	a1, a1, 0xff
 	slli	a3, a1, 8
@@ -48,7 +49,7 @@  WEAK(memset)
 	add	a3, t0, a4
 
 	andi	a4, a4, 31*SZREG	/* Calculate remainder */
-	beqz	a4, 3f			/* Shortcut if no remainder */
+	beqz	a4, .Lduff_loop		/* Shortcut if no remainder */
 	neg	a4, a4
 	addi	a4, a4, 32*SZREG	/* Calculate initial offset */
 
@@ -57,13 +58,13 @@  WEAK(memset)
 
 	/* Jump into loop body */
 	/* Assumes 32-bit instruction lengths */
-	la	a5, 3f
+	la	a5, .Lduff_loop
 #ifdef CONFIG_64BIT
 	srli	a4, a4, 1
 #endif
 	add	a5, a5, a4
 	jr	a5
-3:
+.Lduff_loop:
 	REG_S	a1,        0(t0)
 	REG_S	a1,    SZREG(t0)
 	REG_S	a1,  2*SZREG(t0)
@@ -98,17 +99,17 @@  WEAK(memset)
 	REG_S	a1, 31*SZREG(t0)
 
 	addi	t0, t0, 32*SZREG
-	bltu	t0, a3, 3b
+	bltu	t0, a3, .Lduff_loop
 	andi	a2, a2, SZREG-1		/* Update count */
 
-4:
+.Lfinish:
 	/* Handle trailing misalignment */
-	beqz	a2, 6f
+	beqz	a2, .Ldone
 	add	a3, t0, a2
-5:
+.Lmisaligned2:
 	sb	a1, 0(t0)
 	addi	t0, t0, 1
-	bltu	t0, a3, 5b
-6:
+	bltu	t0, a3, .Lmisaligned2
+.Ldone:
 	ret
 END(__memset)

[8/9] RISC-V: lib: Use named labels in memset

Commit Message

Comments

Patch