Patchwork [U-Boot,5/5] arm: optimize relocate_code routine

login
register
mail settings
Submitter Albert ARIBAUD
Date May 14, 2013, 8:03 p.m.
Message ID <1368561780-19104-6-git-send-email-albert.u.boot@aribaud.net>
Download mbox | patch
Permalink /patch/243817/
State Superseded
Delegated to: Albert ARIBAUD
Headers show

Comments

Albert ARIBAUD - May 14, 2013, 8:03 p.m.
Use section symbols directly
Drop support for R_ARM_ABS32 record types
Eliminate unneeded intermediate registers
Optimize relocation table iteration

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 arch/arm/lib/relocate.S |   45 +++++++++++++++------------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)
Benoît Thébaudeau - May 14, 2013, 11:54 p.m.
Hi Albert,

On Tuesday, May 14, 2013 10:03:00 PM, Albert ARIBAUD wrote:
> Use section symbols directly
> Drop support for R_ARM_ABS32 record types
> Eliminate unneeded intermediate registers
> Optimize relocation table iteration
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>  arch/arm/lib/relocate.S |   45 +++++++++++++++------------------------------
>  1 file changed, 15 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S
> index 818735c..75ee3b4 100644
> --- a/arch/arm/lib/relocate.S
> +++ b/arch/arm/lib/relocate.S
> @@ -37,51 +37,36 @@
>   * This function relocates the monitor code.
>   */
>  ENTRY(relocate_code)
> -	mov	r6, r0	/* save addr of destination */
>  
> -	ldr	r0, =__image_copy_start	/* r0 <- source start address */
> -	subs	r9, r6, r0		/* r9 <- relocation offset */
> +	ldr	r1, =__image_copy_start	/* r1 <- source start address */
> +	subs	r9, r0, r1		/* r9 <- relocation offset */
>  	beq	relocate_done		/* skip relocation */
> -	mov	r1, r6			/* r1 <- scratch for copy loop */
>  	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
>  
>  copy_loop:
> -	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
> -	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
> -	cmp	r0, r2			/* until source end address [r2]    */
> +	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
> +	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
> +	cmp	r1, r2			/* until source end address [r2]    */
>  	blo	copy_loop
>  
>  	/*
>  	 * fix .rel.dyn relocations
>  	 */
> -	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
>  	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
>  	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
>  fixloop:
> -	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
> -	add	r0, r0, r9		/* r0 <- DST location to fix up */
> -	ldr	r1, [r2, #4]
> -	and	r7, r1, #0xff
> -	cmp	r7, #23			/* relative fixup? */
> -	beq	fixrel
> -	cmp	r7, #2			/* absolute fixup? */
> -	beq	fixabs
> -	/* ignore unknown type of fixup */
> -	b	fixnext
> -fixabs:
> -	/* absolute fix: set location to (offset) symbol value */
> -	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
> -	add	r1, r10, r1		/* r1 <- address of symbol in table */
> -	ldr	r1, [r1, #4]		/* r1 <- symbol value */
> -	add	r1, r1, r9		/* r1 <- relocated sym addr */
> -	b	fixnext
> -fixrel:
> +	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
> +	and	r1, r1, #0xff		/* r1 <- fixup type */
> +	cmp	r1, #23			/* relative fixup? */
> +	bne	fixnext
> +
>  	/* relative fix: increase location by offset */
> -	ldr	r1, [r0]
> -	add	r1, r1, r9
> +	add	r0, r0, r9		/* r0 <- DST location to fix up */
> +	ldr	r1, [r0]		/* r1 <- content to fix up */
> +	add	r1, r1, r9		/* fix up */
> +	str	r1, [r0]		/* write back fixed-up content */
> +
>  fixnext:
> -	str	r1, [r0]
> -	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
>  	cmp	r2, r3
>  	blo	fixloop

The final state of relocate.S is correct in this series, but it is not correct
at the end of "[PATCH v2 4/4] arm: factorize relocate_code routine" as I pointed
out in the part of my review that you cut in your first reply.

Best regards,
Benoît
Albert ARIBAUD - May 15, 2013, 7:32 a.m.
Hi Benoît,

On Wed, 15 May 2013 01:54:11 +0200 (CEST), Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> Hi Albert,
> 
> On Tuesday, May 14, 2013 10:03:00 PM, Albert ARIBAUD wrote:
> > Use section symbols directly
> > Drop support for R_ARM_ABS32 record types
> > Eliminate unneeded intermediate registers
> > Optimize relocation table iteration
> > 
> > Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > ---
> >  arch/arm/lib/relocate.S |   45 +++++++++++++++------------------------------
> >  1 file changed, 15 insertions(+), 30 deletions(-)
> > 
> > diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S
> > index 818735c..75ee3b4 100644
> > --- a/arch/arm/lib/relocate.S
> > +++ b/arch/arm/lib/relocate.S
> > @@ -37,51 +37,36 @@
> >   * This function relocates the monitor code.
> >   */
> >  ENTRY(relocate_code)
> > -	mov	r6, r0	/* save addr of destination */
> >  
> > -	ldr	r0, =__image_copy_start	/* r0 <- source start address */
> > -	subs	r9, r6, r0		/* r9 <- relocation offset */
> > +	ldr	r1, =__image_copy_start	/* r1 <- source start address */
> > +	subs	r9, r0, r1		/* r9 <- relocation offset */
> >  	beq	relocate_done		/* skip relocation */
> > -	mov	r1, r6			/* r1 <- scratch for copy loop */
> >  	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
> >  
> >  copy_loop:
> > -	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
> > -	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
> > -	cmp	r0, r2			/* until source end address [r2]    */
> > +	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
> > +	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
> > +	cmp	r1, r2			/* until source end address [r2]    */
> >  	blo	copy_loop
> >  
> >  	/*
> >  	 * fix .rel.dyn relocations
> >  	 */
> > -	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
> >  	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
> >  	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
> >  fixloop:
> > -	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
> > -	add	r0, r0, r9		/* r0 <- DST location to fix up */
> > -	ldr	r1, [r2, #4]
> > -	and	r7, r1, #0xff
> > -	cmp	r7, #23			/* relative fixup? */
> > -	beq	fixrel
> > -	cmp	r7, #2			/* absolute fixup? */
> > -	beq	fixabs
> > -	/* ignore unknown type of fixup */
> > -	b	fixnext
> > -fixabs:
> > -	/* absolute fix: set location to (offset) symbol value */
> > -	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
> > -	add	r1, r10, r1		/* r1 <- address of symbol in table */
> > -	ldr	r1, [r1, #4]		/* r1 <- symbol value */
> > -	add	r1, r1, r9		/* r1 <- relocated sym addr */
> > -	b	fixnext
> > -fixrel:
> > +	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
> > +	and	r1, r1, #0xff		/* r1 <- fixup type */
> > +	cmp	r1, #23			/* relative fixup? */
> > +	bne	fixnext
> > +
> >  	/* relative fix: increase location by offset */
> > -	ldr	r1, [r0]
> > -	add	r1, r1, r9
> > +	add	r0, r0, r9		/* r0 <- DST location to fix up */
> > +	ldr	r1, [r0]		/* r1 <- content to fix up */
> > +	add	r1, r1, r9		/* fix up */
> > +	str	r1, [r0]		/* write back fixed-up content */
> > +
> >  fixnext:
> > -	str	r1, [r0]
> > -	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
> >  	cmp	r2, r3
> >  	blo	fixloop
> 
> The final state of relocate.S is correct in this series, but it is not correct
> at the end of "[PATCH v2 4/4] arm: factorize relocate_code routine" as I pointed
> out in the part of my review that you cut in your first reply.

Error is relocate patch series to be fixed in V2; I'll rebase V2 of
this series accordingly.

Thanks again.

> Best regards,
> Benoît

Amicalement,

Patch

diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S
index 818735c..75ee3b4 100644
--- a/arch/arm/lib/relocate.S
+++ b/arch/arm/lib/relocate.S
@@ -37,51 +37,36 @@ 
  * This function relocates the monitor code.
  */
 ENTRY(relocate_code)
-	mov	r6, r0	/* save addr of destination */
 
-	ldr	r0, =__image_copy_start	/* r0 <- source start address */
-	subs	r9, r6, r0		/* r9 <- relocation offset */
+	ldr	r1, =__image_copy_start	/* r1 <- source start address */
+	subs	r9, r0, r1		/* r9 <- relocation offset */
 	beq	relocate_done		/* skip relocation */
-	mov	r1, r6			/* r1 <- scratch for copy loop */
 	ldr	r2, =__image_copy_end	/* r2 <- source end address	    */
 
 copy_loop:
-	ldmia	r0!, {r10-r11}		/* copy from source address [r0]    */
-	stmia	r1!, {r10-r11}		/* copy to   target address [r1]    */
-	cmp	r0, r2			/* until source end address [r2]    */
+	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
+	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
+	cmp	r1, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
 	/*
 	 * fix .rel.dyn relocations
 	 */
-	ldr	r10, =__dynsym_start	/* r10 <- sym table ofs */
 	ldr	r2, =__rel_dyn_start	/* r2 <- rel dyn start ofs */
 	ldr	r3, =__rel_dyn_end	/* r3 <- rel dyn end ofs */
 fixloop:
-	ldr	r0, [r2]		/* r0 <- SRC location to fix up */
-	add	r0, r0, r9		/* r0 <- DST location to fix up */
-	ldr	r1, [r2, #4]
-	and	r7, r1, #0xff
-	cmp	r7, #23			/* relative fixup? */
-	beq	fixrel
-	cmp	r7, #2			/* absolute fixup? */
-	beq	fixabs
-	/* ignore unknown type of fixup */
-	b	fixnext
-fixabs:
-	/* absolute fix: set location to (offset) symbol value */
-	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
-	add	r1, r10, r1		/* r1 <- address of symbol in table */
-	ldr	r1, [r1, #4]		/* r1 <- symbol value */
-	add	r1, r1, r9		/* r1 <- relocated sym addr */
-	b	fixnext
-fixrel:
+	ldmia	r2!, {r0-r1}		/* (r0,r1) <- (SRC location,fixup) */
+	and	r1, r1, #0xff		/* r1 <- fixup type */
+	cmp	r1, #23			/* relative fixup? */
+	bne	fixnext
+
 	/* relative fix: increase location by offset */
-	ldr	r1, [r0]
-	add	r1, r1, r9
+	add	r0, r0, r9		/* r0 <- DST location to fix up */
+	ldr	r1, [r0]		/* r1 <- content to fix up */
+	add	r1, r1, r9		/* fix up */
+	str	r1, [r0]		/* write back fixed-up content */
+
 fixnext:
-	str	r1, [r0]
-	add	r2, r2, #8		/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
 	blo	fixloop