On Mon, Jun 12, 2023 at 4:03 PM Gedare Bloom <ged...@rtems.org> wrote: > > On Mon, Jun 12, 2023 at 3:54 PM Joel Sherrill <j...@rtems.org> wrote: > > > > > > > > On Mon, Jun 12, 2023 at 4:50 PM Gedare Bloom <ged...@rtems.org> wrote: > >> > >> On Mon, Jun 12, 2023 at 2:28 PM Joel Sherrill <j...@rtems.org> wrote: > >> > > >> > > >> > > >> > On Mon, Jun 12, 2023 at 3:10 PM Gedare Bloom <ged...@rtems.org> wrote: > >> >> > >> >> We have a mix of ways to write inline assembly. It would be convenient > >> >> to choose one way. The prevailing options are based on breaking around > >> >> the colons (reg/field separators), either to break at colons if the > >> >> line length > 80, or to always break at colons. > >> >> > >> >> I personally find it is easier to read inline assembly that has broken > >> >> at the colons. But we have these options: > >> >> 1. Always break at the colon > >> >> 2. Only break when the line length is exceeded > >> > > >> > > >> > I lean to (1). > >> >> > >> >> > >> >> With #2, we also can break as usual, or we can force breaks at the > >> >> colons. I have seen examples of both in the source tree. Any strong > >> >> opinions one way or another? > >> > > >> > > >> > There may also be cases of multiple assembly instructions on the > >> > same line or split across multiple with line continuation. I don't know > >> > of one offhand though. > >> A complicated example of that would be > >> https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/armv7m-exception-default.c#n60 > >> > > > > Yikes! Does that look ok after clang-format processes it? > > > Yes. > > Below is a snippet of patch for that. One thing to note, there's > currently no way to avoid adding the spaces inside of > '__atribute__((naked))' as a special case. If this is desired, it will > need to be added to clang-format probably. > > diff --git a/cpukit/score/cpu/arm/armv7m-exception-default.c > b/cpukit/score/cpu/arm/armv7m-exception-default.c > index 35dde50dc3..ff00ad2a72 100644 > --- a/cpukit/score/cpu/arm/armv7m-exception-default.c > +++ b/cpukit/score/cpu/arm/armv7m-exception-default.c > @@ -42,23 +42,23 @@ > > #ifdef ARM_MULTILIB_ARCH_V7M > > -void __attribute__((naked)) _ARMV7M_Exception_default( void ) > +void __attribute__( ( naked ) ) _ARMV7M_Exception_default( void ) > { > - /* On exception entry, ARMv7M saves context state onto a stack pointed to > - * by either MSP or PSP. The value stored in LR indicates whether we were > - * in Thread or Handler mode, whether we were using the FPU (if any), > - * and which stack pointer we were using. > - * In particular, bit 2 of LR will be 0 if we were using MSP. > - * > - * For a more detailed explanation, see the Exception Entry Behavior > - * section of the ARMv7M Architecture Reference Manual. > - */ > + /* On exception entry, ARMv7M saves context state onto a stack pointed to > + * by either MSP or PSP. The value stored in LR indicates whether we were > + * in Thread or Handler mode, whether we were using the FPU (if any), > + * and which stack pointer we were using. > + * In particular, bit 2 of LR will be 0 if we were using MSP. > + * > + * For a more detailed explanation, see the Exception Entry Behavior > + * section of the ARMv7M Architecture Reference Manual. > + */ > > - /* As we're in Handler mode here, we'll always operate on MSP. > - * However, we need to store the right SP in our CPU_Exception_frame. > - */ > - __asm__ volatile ( > - "sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame. */ > + /* As we're in Handler mode here, we'll always operate on MSP. > + * However, we need to store the right SP in our CPU_Exception_frame. > + */ > + __asm__ volatile(
One other note, there's currently no way to force a space after volatile. Again if that's something we want to preserve, it probably needs to be added somehow. > + "sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame. */ > "stm sp, {r0-r12}\n" > "tst lr, #4\n" /* Check if bit 2 of LR is zero. If so, > PSR.Z = 1 */ > "itte eq\n" /* IF bit 2 of LR is zero... (PSR.Z == 1) */ > @@ -67,15 +67,15 @@ void __attribute__((naked)) > _ARMV7M_Exception_default( void ) > "mrsne r3, psp\n" /* ELSE it is not zero; we were using PSP */ > "add r2, r3, %[v7mlroff]\n" > "add r1, sp, %[cpuspoff]\n" > - "ldm r2, {r4-r6}\n" /* Grab LR, PC and xPSR from the stack */ > - "tst lr, #16\n" /* Check if we have an FP state on the frame */ > + "ldm r2, {r4-r6}\n" /* Grab LR, PC and xPSR from the stack */ > + "tst lr, #16\n" /* Check if we have an FP state on the frame */ > "ite eq\n" > - "addeq r3, #104\n" /* Back to previous SP with FP state */ > - "addne r3, #32\n" /* Back to previous SP without FP state */ > - "tst r6, #512\n" /* Check xPSR if the SP was aligned */ > + "addeq r3, #104\n" /* Back to previous SP with FP state */ > + "addne r3, #32\n" /* Back to previous SP without FP state */ > + "tst r6, #512\n" /* Check xPSR if the SP was aligned */ > "it ne\n" > - "addne r3, #4\n" /* Undo alignment */ > - "stm r1, {r3-r6}\n" /* Store to CPU_Exception_frame */ > + "addne r3, #4\n" /* Undo alignment */ > + "stm r1, {r3-r6}\n" /* Store to CPU_Exception_frame */ > "mrs r1, ipsr\n" > "str r1, [sp, %[cpuvecoff]]\n" > > @@ -113,13 +113,13 @@ void __attribute__((naked)) > _ARMV7M_Exception_default( void ) > > "b _ARM_Exception_default\n" > : > - : [cpufsz] "i" (sizeof(CPU_Exception_frame)), > - [cpuspoff] "i" (offsetof(CPU_Exception_frame, register_sp)), > - [v7mlroff] "i" (offsetof(ARMV7M_Exception_frame, register_lr)), > - [cpuvecoff] "J" (offsetof(CPU_Exception_frame, vector)), > - [cpuvfpoff] "i" (ARM_EXCEPTION_FRAME_VFP_CONTEXT_OFFSET), > - [cpacr] "i" (ARMV7M_CPACR), > - [vfpsz] "i" (ARM_VFP_CONTEXT_SIZE) > + : [cpufsz] "i"( sizeof( CPU_Exception_frame ) ), > + [cpuspoff] "i"( offsetof( CPU_Exception_frame, register_sp ) ), > + [v7mlroff] "i"( offsetof( ARMV7M_Exception_frame, register_lr ) ), > + [cpuvecoff] "J"( offsetof( CPU_Exception_frame, vector ) ), > + [cpuvfpoff] "i"( ARM_EXCEPTION_FRAME_VFP_CONTEXT_OFFSET ), > + [cpacr] "i"( ARMV7M_CPACR ), > + [vfpsz] "i"( ARM_VFP_CONTEXT_SIZE ) > ); > } > > > >> > >> >> > >> >> > >> >> here's a line broken because of line lengths, that has not split the > >> >> arguments at the colons: > >> >> https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/include/rtems/score/aarch32-system-registers.h#n69 > >> >> > >> >> Here's a line broken because of line lengths: > >> >> https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/include/rtems/score/cpu.h#n501 > >> >> > >> >> Here's a line broken always: > >> >> https://git.rtems.org/rtems/tree/cpukit/score/cpu/aarch64/cpu.c#n153 > >> >> > >> >> And for good measure, here's an unbroken line that should be broken: > >> >> https://git.rtems.org/rtems/tree/cpukit/score/cpu/microblaze/include/rtems/score/cpu.h#n206 > >> >> > >> >> With the newest version of clang-format we will be able to accommodate > >> >> always breaking the lines. It currently is inconsistent with whether > >> >> it puts the first argument on its own line, or keeps it with the > >> >> "__asm__ volatile (" that I could probably make consistent if we > >> >> decide we need it to be. > >> >> > >> >> Gedare > >> >> _______________________________________________ > >> >> devel mailing list > >> >> devel@rtems.org > >> >> http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel