Re: [PATCH] EmbeddedPkg/GdbDebugAgent ARM: use modern dialect for ldm/stm instructions

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Wed, 12 Dec 2018 at 12:30, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Wed, Dec 12, 2018 at 10:12:11AM +0100, Ard Biesheuvel wrote:
> > Clang does not like the legacy 32-bit assembler syntax used in the
> > gdbstub exception handling routines, so update them to something
> > more fashionable.
> >
> > So switch the order of the conditional suffix
>
> Yes, this is essentially a bugfix. The syntax change predates ARM's
> involvement in EDK2.
>
> > and the increment/decrement
> > specifier, and use decrement-after (da) and increment-before (ib) as
> > appropriate for an empty descending stack.
>
> But I very much prefer the FA/FD/EA/ED syntax. It gives symmetry,
> which helps me a lot when dealing with 7-character mnemonics.
> These aliases are also defined in the ARM ARM and won't be going away.
>

Clang doesn't support them though.

This code is not used anywhere (it is referenced in BeagleBoardPkg but
commented out), so we could also simply remove it.

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  EmbeddedPkg/Library/GdbDebugAgent/Arm/ExceptionSupport.ARMv6.S | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/EmbeddedPkg/Library/GdbDebugAgent/Arm/ExceptionSupport.ARMv6.S b/EmbeddedPkg/Library/GdbDebugAgent/Arm/ExceptionSupport.ARMv6.S
> > index df1543a6d242..924578c7af2f 100644
> > --- a/EmbeddedPkg/Library/GdbDebugAgent/Arm/ExceptionSupport.ARMv6.S
> > +++ b/EmbeddedPkg/Library/GdbDebugAgent/Arm/ExceptionSupport.ARMv6.S
> > @@ -57,6 +57,7 @@ GCC_ASM_EXPORT(AsmCommonExceptionEntry)
> >  GCC_ASM_EXPORT(GdbExceptionHandler)
> >
> >  .text
> > +.syntax unified
> >  .align 3
> >
> >
> > @@ -198,9 +199,9 @@ ASM_PFX(AsmCommonExceptionEntry):
> >    and       R3, R1, #0x1f           @ Check CPSR to see if User or System Mode
> >    cmp       R3, #0x1f               @ if ((CPSR == 0x10) || (CPSR == 0x1df))
> >    cmpne     R3, #0x10               @
> > -  stmeqed   R2, {lr}^               @   save unbanked lr
> > +  stmdaeq   R2, {lr}^               @   save unbanked lr
> >                                      @ else
> > -  stmneed   R2, {lr}                @   save SVC lr
> > +  stmdane   R2, {lr}                @   save SVC lr
> >
> >
> >    ldr       R5, [SP, #0x58]         @ PC is the LR pushed by srsfd
> > @@ -245,9 +246,9 @@ GdbExceptionHandler (
> >    and       R1, R1, #0x1f           @ Check to see if User or System Mode
> >    cmp       R1, #0x1f               @ if ((CPSR == 0x10) || (CPSR == 0x1f))
> >    cmpne     R1, #0x10               @
> > -  ldmeqed   R2, {lr}^               @   restore unbanked lr
> > +  ldmibeq   R2, {lr}^               @   restore unbanked lr
> >                                      @ else
> > -  ldmneed   R3, {lr}                @   restore SVC lr, via ldmfd SP!, {LR}
> > +  ldmibne   R3, {lr}                @   restore SVC lr, via ldmfd SP!, {LR}
> >
> >    ldmfd     SP!,{R0-R12}            @ Restore general purpose registers
> >                                      @ Exception handler can not change SP
> > --
> > 2.19.2
> >