[U-Boot] [PATCH] arm: New stack pointer is already aligned

[U-Boot] [PATCH] arm: New stack pointer is already aligned

[Tom Rini]

The code in arch/arm/lib/board.c::board_init_f that sets
gd->start_addr_sp has already make sure we're 8-byte aligned, so we
don't need to do that again.

Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/lib/crt0.S |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index ac54b93..6b5ec01 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -82,7 +82,6 @@ ENTRY(_main)
  */
 
 	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
 	ldr	r9, [r9, #GD_BD]		/* r9 = gd->bd */
 	sub	r9, r9, #GD_SIZE		/* new GD is below bd */
 
-- 
1.7.9.5

[U-Boot] [PATCH] arm: New stack pointer is already aligned

[Wolfgang Denk]

Dear Tom Rini,

In message <1385580930-9830-1-git-send-email-trini@ti.com> you wrote:
> The code in arch/arm/lib/board.c::board_init_f that sets
> gd->start_addr_sp has already make sure we're 8-byte aligned, so we
> don't need to do that again.
> 
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/lib/crt0.S |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> index ac54b93..6b5ec01 100644
> --- a/arch/arm/lib/crt0.S
> +++ b/arch/arm/lib/crt0.S
> @@ -82,7 +82,6 @@ ENTRY(_main)
>   */
>  
>  	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
> -	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
>  	ldr	r9, [r9, #GD_BD]		/* r9 = gd->bd */
>  	sub	r9, r9, #GD_SIZE		/* new GD is below bd */

I recommend to keep this instruction. It's just a bit of defensive
programming, and removing it does not save any measurable amount of
memory footprint nor execution time.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Madness takes its toll. Please have exact change.

[U-Boot] [PATCH] arm: New stack pointer is already aligned

[Albert ARIBAUD]

Hi Wolfgang,

On Fri, 29 Nov 2013 07:14:13 +0100, Wolfgang Denk <wd@denx.de> wrote:

> Dear Tom Rini,
> 
> In message <1385580930-9830-1-git-send-email-trini@ti.com> you wrote:
> > The code in arch/arm/lib/board.c::board_init_f that sets
> > gd->start_addr_sp has already make sure we're 8-byte aligned, so we
> > don't need to do that again.
> > 
> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > Signed-off-by: Tom Rini <trini@ti.com>
> > ---
> >  arch/arm/lib/crt0.S |    1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> > index ac54b93..6b5ec01 100644
> > --- a/arch/arm/lib/crt0.S
> > +++ b/arch/arm/lib/crt0.S
> > @@ -82,7 +82,6 @@ ENTRY(_main)
> >   */
> >  
> >  	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
> > -	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
> >  	ldr	r9, [r9, #GD_BD]		/* r9 = gd->bd */
> >  	sub	r9, r9, #GD_SIZE		/* new GD is below bd */
> 
> I recommend to keep this instruction. It's just a bit of defensive
> programming, and removing it does not save any measurable amount of
> memory footprint nor execution time.

I would even go further: it is the setting of SP in C code which
should not be kept. I doubt the C specification mentions what should /
might happen when changing the stack pointer on the fly in code which
might need the stack at any point.
 
> Best regards,
> 
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] arm: New stack pointer is already aligned

[Tom Rini]

On Mon, Jan 13, 2014 at 10:07:50AM +0100, Albert ARIBAUD wrote:
> Hi Wolfgang,
> 
> On Fri, 29 Nov 2013 07:14:13 +0100, Wolfgang Denk <wd@denx.de> wrote:
> 
> > Dear Tom Rini,
> > 
> > In message <1385580930-9830-1-git-send-email-trini@ti.com> you wrote:
> > > The code in arch/arm/lib/board.c::board_init_f that sets
> > > gd->start_addr_sp has already make sure we're 8-byte aligned, so we
> > > don't need to do that again.
> > > 
> > > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > > Signed-off-by: Tom Rini <trini@ti.com>
> > > ---
> > >  arch/arm/lib/crt0.S |    1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> > > index ac54b93..6b5ec01 100644
> > > --- a/arch/arm/lib/crt0.S
> > > +++ b/arch/arm/lib/crt0.S
> > > @@ -82,7 +82,6 @@ ENTRY(_main)
> > >   */
> > >  
> > >  	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
> > > -	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
> > >  	ldr	r9, [r9, #GD_BD]		/* r9 = gd->bd */
> > >  	sub	r9, r9, #GD_SIZE		/* new GD is below bd */
> > 
> > I recommend to keep this instruction. It's just a bit of defensive
> > programming, and removing it does not save any measurable amount of
> > memory footprint nor execution time.
> 
> I would even go further: it is the setting of SP in C code which
> should not be kept. I doubt the C specification mentions what should /
> might happen when changing the stack pointer on the fly in code which
> might need the stack at any point.

I'm fine with dropping this patch (I noticed as I was tracing something
else).  But please note that in C we're just setting gd->start_addr_sp
and aligning it there, we don't modify sp at all until the snippet of
asm above.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140113/45d38bdc/attachment.pgp>