[U-Boot] [PATCH] 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 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 */
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
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,
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.
participants (3)
-
Albert ARIBAUD -
Tom Rini -
Wolfgang Denk