Re: [Arm] Align comments and spaces in crt0.S libgloss and sys/arm
Alexander Fedotov <[email protected]>
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <CAN8C2CqShYDiHsZBoL+eT-P5YZBqTorKkm9SkFj5Ud2geEOrkQ@mail.gmail.com> |
>Style rules say that the full stop goes inside the parenthesis if the >entire sentence is parenthetical; otherwise it goes outside. So its >technically the other version that is incorrect. Well I just wanted to have the same style in both sources :) I have a new patch for SP&SL refactor that is based on this one. >Do you have commit rights? Unfortunately no Alex On Thu, Apr 11, 2019 at 4:33 PM Richard Earnshaw (lists) <[email protected]> wrote: > > On 11/04/2019 13:58, Alexander Fedotov wrote: > > Sorry. I have missed some lines in previous patch. Here is new one below. > > > > From 68d7a9bb5f11ddc3c0420560c718b6a2f8041896 Mon Sep 17 00:00:00 2001 > > From: Alexander Fedotov <[email protected]> > > Date: Thu, 11 Apr 2019 15:28:05 +0300 > > Subject: [PATCH] Align comments and spaces in libgloss/arm/crt0.S and > > newlib/libc/sys/arm/crt0.S to ease further code alignment. > > > > --- > > libgloss/arm/crt0.S | 131 ++++++++++++++++++------------------- > > newlib/libc/sys/arm/crt0.S | 12 ++-- > > 2 files changed, 71 insertions(+), 72 deletions(-) > > > > diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S > > index c708f63d8..e5c813e3b 100644 > > --- a/libgloss/arm/crt0.S > > +++ b/libgloss/arm/crt0.S > > @@ -90,14 +90,14 @@ > > #endif > > #endif > > > > -/* Start by setting up a stack */ > > +/* Start by setting up a stack. */ > > #ifdef ARM_RDP_MONITOR > > - /* Issue Demon SWI to read stack info */ > > - swi SWI_GetEnv /* Returns command line in r0 */ > > - mov sp,r1 /* and the highest memory address in r1 */ > > + /* Issue Demon SWI to read stack info. */ > > + swi SWI_GetEnv /* Returns command line in r0. */ > > + mov sp,r1 /* and the highest memory address in r1. */ > > > > - /* stack limit is at end of data */ > > - /* allow slop for stack overflow handling and small frames */ > > + /* Stack limit is at end of data. */ > > + /* Allow slop for stack overflow handling and small frames. */ > > #ifdef THUMB1_ONLY > > ldr r0, .LC2 > > adds r0, #128 > > @@ -109,19 +109,19 @@ > > #endif > > #else > > #ifdef ARM_RDI_MONITOR > > - /* Issue Angel SWI to read stack info */ > > + /* Issue Angel SWI to read stack info. */ > > movs r0, #AngelSWI_Reason_HeapInfo > > - adr r1, .LC0 /* point at ptr to 4 words to receive data */ > > + adr r1, .LC0 /* Point at ptr to 4 words to receive data. */ > > #ifdef THUMB_VXM > > bkpt AngelSWI > > #elif defined(__thumb2__) > > - /* We are in thumb mode for startup on armv7 architectures. */ > > + /* We are in thumb mode for startup on armv7 architectures. */ > > AngelSWIAsm (AngelSWI) > > #else > > - /* We are always in ARM mode for startup on pre armv7 archs. */ > > + /* We are always in ARM mode for startup on pre armv7 archs. */ > > AngelSWIAsm (AngelSWI_ARM) > > #endif > > - ldr r0, .LC0 /* point at values read */ > > + ldr r0, .LC0 /* Point at values read. */ > > > > /* Set __heap_limit. */ > > ldr r1, [r0, #4] > > @@ -148,14 +148,15 @@ > > to skip setting sp/sl to 0 here. > > - Considering M-profile processors, We might want to initialize > > sp by the first entry of vector table and return 0 to SYS_HEAPINFO > > - semihosting call, which will be skipped here. */ > > + semihosting call, which will be skipped here. */ > > cmp r1, #0 > > beq .LC26 > > mov sp, r1 > > .LC26: > > cmp r2, #0 > > beq .LC27 > > - /* allow slop for stack overflow handling and small frames */ > > + > > + /* Allow slop for stack overflow handling and small frames. */ > > #ifdef THUMB1_ONLY > > adds r2, #128 > > adds r2, #128 > > @@ -163,9 +164,10 @@ > > #else > > add sl, r2, #256 > > #endif > > + > > .LC27: > > #else > > - /* Set up the stack pointer to a fixed value */ > > + /* Set up the stack pointer to a fixed value. */ > > /* Changes by toralf: > > - Allow linker script to provide stack via __stack symbol - see > > defintion of .Lstack > > @@ -175,7 +177,7 @@ > > Loosely based on init.s from ARM/Motorola example code. > > Note: Mode switch via CPSR is not allowed once in non-privileged > > mode, so we take care not to enter "User" to set up its sp, > > - and also skip most operations if already in that mode. */ > > + and also skip most operations if already in that mode. */ > > > > ldr r3, .Lstack > > cmp r3, #0 > > @@ -192,42 +194,42 @@ > > /* Note: This 'mov' is essential when starting in User, and ensures we > > always get *some* sp value for the initial mode, even if we > > have somehow missed it below (in which case it gets the same > > - value as FIQ - not ideal, but better than nothing.) */ > > + value as FIQ - not ideal, but better than nothing). */ > > mov sp, r3 > > #ifdef PREFER_THUMB > > /* XXX Fill in stack assignments for interrupt modes. */ > > #else > > mrs r2, CPSR > > - tst r2, #0x0F /* Test mode bits - in User of all are 0 */ > > - beq .LC23 /* "eq" means r2 AND #0x0F is 0 */ > > - msr CPSR_c, #0xD1 /* FIRQ mode, interrupts disabled */ > > + tst r2, #0x0F /* Test mode bits - in User of all are 0. */ > > + beq .LC23 /* "eq" means r2 AND #0x0F is 0. */ > > + msr CPSR_c, #0xD1 /* FIRQ mode, interrupts disabled. */ > > mov sp, r3 > > - sub sl, sp, #0x1000 /* This mode also has its own sl (see below) */ > > + sub sl, sp, #0x1000 /* This mode also has its own sl (see below). */ > > > > mov r3, sl > > - msr CPSR_c, #0xD7 /* Abort mode, interrupts disabled */ > > + msr CPSR_c, #0xD7 /* Abort mode, interrupts disabled. */ > > mov sp, r3 > > sub r3, r3, #0x1000 > > > > - msr CPSR_c, #0xDB /* Undefined mode, interrupts disabled */ > > + msr CPSR_c, #0xDB /* Undefined mode, interrupts disabled. */ > > mov sp, r3 > > sub r3, r3, #0x1000 > > > > - msr CPSR_c, #0xD2 /* IRQ mode, interrupts disabled */ > > + msr CPSR_c, #0xD2 /* IRQ mode, interrupts disabled. */ > > mov sp, r3 > > sub r3, r3, #0x2000 > > > > - msr CPSR_c, #0xD3 /* Supervisory mode, interrupts disabled */ > > + msr CPSR_c, #0xD3 /* Supervisory mode, interrupts disabled. */ > > > > mov sp, r3 > > - sub r3, r3, #0x8000 /* Min size 32k */ > > - bic r3, r3, #0x00FF /* Align with current 64k block */ > > + sub r3, r3, #0x8000 /* Min size 32k. */ > > + bic r3, r3, #0x00FF /* Align with current 64k block. */ > > bic r3, r3, #0xFF00 > > > > str r3, [r3, #-4] /* Move value into user mode sp without */ > > - ldmdb r3, {sp}^ /* changing modes, via '^' form of ldm */ > > + ldmdb r3, {sp}^ /* changing modes, via '^' form of ldm. */ > > orr r2, r2, #0xC0 /* Back to original mode, presumably SVC, */ > > - msr CPSR_c, r2 /* with FIQ/IRQ disable bits forced to 1 */ > > + msr CPSR_c, r2 /* with FIQ/IRQ disable bits forced to 1. */ > > #endif > > .LC23: > > /* Setup a default stack-limit in-case the code has been > > @@ -243,24 +245,24 @@ > > subs r2, r3, r2 > > mov sl, r2 > > #else > > - sub sl, r3, #64 << 10 /* Still assumes 256bytes below sl */ > > + sub sl, r3, #64 << 10 /* Still assumes 256bytes below sl. */ > > #endif > > #endif > > #endif > > /* Zero the memory in the .bss section. */ > > - movs a2, #0 /* Second arg: fill value */ > > - mov fp, a2 /* Null frame pointer */ > > - mov r7, a2 /* Null frame pointer for Thumb */ > > + movs a2, #0 /* Second arg: fill value. */ > > + mov fp, a2 /* Null frame pointer. */ > > + mov r7, a2 /* Null frame pointer for Thumb. */ > > > > - ldr a1, .LC1 /* First arg: start of memory block */ > > + ldr a1, .LC1 /* First arg: start of memory block. */ > > ldr a3, .LC2 > > - subs a3, a3, a1 /* Third arg: length of block */ > > + subs a3, a3, a1 /* Third arg: length of block. */ > > > > > > #if __thumb__ && !defined(PREFER_THUMB) > > - /* Enter Thumb mode.... */ > > - add a4, pc, #1 /* Get the address of the Thumb block */ > > - bx a4 /* Go there and start Thumb decoding */ > > + /* Enter Thumb mode... */ > > + add a4, pc, #1 /* Get the address of the Thumb block. */ > > + bx a4 /* Go there and start Thumb decoding. */ > > > > .code 16 > > .global __change_mode > > @@ -271,9 +273,8 @@ __change_mode: > > bl FUNCTION (memset) > > #if !defined (ARM_RDP_MONITOR) && !defined (ARM_RDI_MONITOR) > > /* Changes by toralf: Taken from libgloss/m68k/crt0.S > > - * initialize target specific stuff. Only execute these > > - * functions it they exist. > > - */ > > + initialize target specific stuff. Only execute these > > + functions it they exist. */ > > ldr r3, .Lhwinit > > cmp r3, #0 > > beq .LC24 > > @@ -285,24 +286,24 @@ __change_mode: > > indirect_call r3 > > > > .LC25: > > - movs r0, #0 /* no arguments */ > > - movs r1, #0 /* no argv either */ > > + movs r0, #0 /* No arguments. */ > > + movs r1, #0 /* No argv either. */ > > #else > > - /* Need to set up standard file handles */ > > + /* Need to set up standard file handles. */ > > bl FUNCTION (initialise_monitor_handles) > > > > #ifdef ARM_RDP_MONITOR > > - swi SWI_GetEnv /* sets r0 to point to the command line */ > > + swi SWI_GetEnv /* Sets r0 to point to the command line. */ > > movs r1, r0 > > #else > > movs r0, #AngelSWI_Reason_GetCmdLine > > - ldr r1, .LC30 /* Space for command line */ > > + ldr r1, .LC30 /* Space for command line. */ > > AngelSWIAsm (AngelSWI) > > ldr r1, .LC30 > > ldr r1, [r1] > > #endif > > - /* Parse string at r1 */ > > - movs r0, #0 /* count of arguments so far */ > > + /* Parse string at r1. */ > > + movs r0, #0 /* Count of arguments so far. */ > > /* Push a NULL argument onto the end of the list. */ > > #ifdef __thumb__ > > push {r0} > > @@ -310,7 +311,7 @@ __change_mode: > > stmfd sp!, {r0} > > #endif > > .LC10: > > -/* Skip leading blanks */ > > +/* Skip leading blanks. */ > > #ifdef __thumb__ > > ldrb r3, [r1] > > adds r1, #1 > > @@ -322,7 +323,7 @@ __change_mode: > > cmp r3, #' ' > > beq .LC10 > > > > -/* See whether we are scanning a string */ > > +/* See whether we are scanning a string. */ > > cmp r3, #'"' > > #ifdef __thumb__ > > beq .LC20 > > @@ -333,17 +334,17 @@ __change_mode: > > b .LC22 > > > > .LC21: > > - movs r2, #' ' /* terminator type */ > > - subs r1, r1, #1 /* adjust back to point at start char */ > > + movs r2, #' ' /* Terminator type. */ > > + subs r1, r1, #1 /* Adjust back to point at start char. */ > > .LC22: > > #else > > cmpne r3, #'\'' > > moveq r2, r3 > > - movne r2, #' ' /* terminator type */ > > - subne r1, r1, #1 /* adjust back to point at start char */ > > + movne r2, #' ' /* Terminator type. */ > > + subne r1, r1, #1 /* Adjust back to point at start char. */ > > #endif > > > > -/* Stack a pointer to the current argument */ > > +/* Stack a pointer to the current argument. */ > > #ifdef __thumb__ > > push {r1} > > #else > > @@ -359,16 +360,16 @@ __change_mode: > > #endif > > cmp r3, #0 > > beq .LC12 > > - cmp r2, r3 /* reached terminator? */ > > + cmp r2, r3 /* Reached terminator ? */ > > bne .LC11 > > movs r2, #0 > > subs r3, r1, #1 > > - strb r2, [r3] /* terminate the arg string */ > > + strb r2, [r3] /* Terminate the arg string. */ > > b .LC10 > > > > .LC12: > > - mov r1, sp /* point at stacked arg pointers */ > > - /* We've now got the stacked args in order reverse the */ > > + mov r1, sp /* Point at stacked arg pointers. */ > > + /* We've now got the stacked args in order, reverse them. */ > > #ifdef __thumb__ > > movs r2, r0 > > lsls r2, #2 > > @@ -390,10 +391,10 @@ __change_mode: > > bics r4, r5 > > mov sp, r4 > > #else > > - add r2, sp, r0, LSL #2 /* End of args */ > > - mov r3, sp /* Start of args */ > > + add r2, sp, r0, LSL #2 /* End of args. */ > > + mov r3, sp /* Start of args. */ > > .LC13: cmp r2, r3 > > - ldrhi r4,[r2, #-4] /* Reverse ends of list */ > > + ldrhi r4,[r2, #-4] /* Reverse ends of list. */ > > ldrhi r5, [r3] > > strhi r5, [r2, #-4]! > > strhi r4, [r3], #4 > > @@ -431,7 +432,6 @@ __change_mode: > > > > #if __thumb__ && !defined(PREFER_THUMB) > > /* Come out of Thumb mode. This code should be redundant. */ > > - > > mov a4, pc > > bx a4 > > > > @@ -447,7 +447,6 @@ change_back: > > > > /* For Thumb, constants must be after the code since only > > positive offsets are supported for PC relative addresses. */ > > - > > .align 0 > > .LC0: > > #ifdef ARM_RDI_MONITOR > > @@ -457,13 +456,13 @@ change_back: > > /* Changes by toralf: Provide alternative "stack" variable whose value > > may be defined externally; .Lstack will be used instead of .LC0 if > > it points to a non-0 value. Also set up references to "hooks" that > > - may be used by the application to provide additional init code. */ > > - > > + may be used by the application to provide additional init code. */ > > #ifdef __pe__ > > .word 0x800000 > > #else > > .word 0x80000 /* Top of RAM on the PIE board. */ > > #endif > > + > > .Lstack: > > .word __stack > > .Lhwinit: > > @@ -476,7 +475,7 @@ change_back: > > runtime (meaning "ignore setting") for the variables, when the user > > does not provide the symbols. (The linker uses a weak symbol if, > > and only if, a normal version of the same symbol isn't provided > > - e.g. by a linker script or another object file.) */ > > + e.g. by a linker script or another object file). */ > > Style rules say that the full stop goes inside the parenthesis if the > entire sentence is parenthetical; otherwise it goes outside. So its > technically the other version that is incorrect. > > > > > .weak __stack > > .weak FUNCTION (hardware_init_hook) > > diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S > > index 64d425900..602148969 100644 > > --- a/newlib/libc/sys/arm/crt0.S > > +++ b/newlib/libc/sys/arm/crt0.S > > @@ -50,13 +50,13 @@ > > .global \name > > .thumb_func > > \name: > > -.endm > > +.endm > > #else > > .code 32 > > .macro FUNC_START name > > - .global \name > > + .global \name > > \name: > > -.endm > > +.endm > > #endif > > > > .macro indirect_call reg > > @@ -399,7 +399,7 @@ __change_mode: > > bl FUNCTION (_init) > > movs r0, r4 > > movs r1, r5 > > -#endif > > +#endif > > bl FUNCTION (main) > > > > bl FUNCTION (exit) /* Should not return. */ > > @@ -420,7 +420,7 @@ change_back: > > #endif > > > > /* For Thumb, constants must be after the code since only > > - positive offsets are supported for PC relative addresses. */ > > + positive offsets are supported for PC relative addresses. */ > > .align 0 > > .LC0: > > #ifdef ARM_RDI_MONITOR > > @@ -430,7 +430,7 @@ change_back: > > /* Changes by toralf: Provide alternative "stack" variable whose value > > may be defined externally; .Lstack will be used instead of .LC0 if > > it points to a non-0 value. Also set up references to "hooks" that > > - may be used by the application to provide additional init > > code. */ > > + may be used by the application to provide additional init code. */ > > #ifdef __pe__ > > .word 0x800000 > > #else > > > > Other than the comment above, this is fine. Thanks for doing this. > > Do you have commit rights? > > R.