arm: New stack pointer is already aligned

PATCH New Tom Rini 3 participants 4 messages Nov 27, 2013 – Jan 13, 2014 Arm (32-bit)

Share & tools

Tom Rini

author

2013-11-27 19:35 UTC #0 lore raw

PATCH 1/1 arm: New stack pointer is already aligned

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 <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/lib/crt0.S |    1 -
 1 file changed, 1 deletion(-)

arch/arm/lib/crt0.S +0 -1

index ac54b93..6b5ec01 100644
@@ -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 */

Signature · 1 line

1.7.9.5

Wolfgang Denk

2013-11-29 06:14 UTC #1 lore raw

PATCH 1/1 arm: New stack pointer is already aligned

Dear Tom Rini,

Quoted 15 lines · In message <[email protected]> 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 arch/arm/lib/crt0.S |    1 -
 1 file changed, 1 deletion(-)
arch/arm/lib/crt0.S +0 -0
index ac54b93..6b5ec01 100644

arch/arm/lib/crt0.S ENTRY(_main) · line 86

Quoted 6 lines

arch/arm/lib/crt0.S +0 -1

@@ -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

Signature · 4 lines

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.

Albert ARIBAUD

2014-01-13 09:07 UTC #2 lore raw

PATCH 1/1 arm: New stack pointer is already aligned

Hi Wolfgang,

Quoted 20 lines · Wolfgang Denk

Dear Tom Rini,
Quoted 17 lines · In message <[email protected]> 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 arch/arm/lib/crt0.S |    1 -
 1 file changed, 1 deletion(-)
arch/arm/lib/crt0.S +0 -0
index ac54b93..6b5ec01 100644
@@ -82,7 +82,6 @@ ENTRY(_main)
  */
 

Quoted 8 lines

arch/arm/lib/crt0.S ENTRY(_main) · line 86
Quoted 4 lines
arch/arm/lib/crt0.S +0 -1
 	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.

Quoted 3 lines

Best regards,
Wolfgang Denk

Amicalement,

Signature · 1 line

Albert.

Tom Rini

author

2014-01-13 13:40 UTC #3 lore raw

PATCH 1/1 arm: New stack pointer is already aligned

Quoted 33 lines · Albert ARIBAUD

Hi Wolfgang,
Quoted 20 lines · Wolfgang Denk
Dear Tom Rini,
Quoted 17 lines · In message <[email protected]> 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 <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 arch/arm/lib/crt0.S |    1 -
 1 file changed, 1 deletion(-)
arch/arm/lib/crt0.S +0 -0
index ac54b93..6b5ec01 100644
@@ -82,7 +82,6 @@ ENTRY(_main)
  */
 
Quoted 8 lines
arch/arm/lib/crt0.S ENTRY(_main) · line 86
Quoted 4 lines
arch/arm/lib/crt0.S +0 -1
 	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.

Quoted 4 lines

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.

Signature · 8 lines

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>