On Wed, May 21, 2014 at 9:53 AM, Sebastian Huber <sebastian.hu...@embedded-brains.de> wrote: > On 2014-05-21 15:44, Gedare Bloom wrote: >> >> On Wed, May 21, 2014 at 4:37 AM, Daniel Hellstrom <dan...@gaisler.com> >> wrote: >>> >>> On 05/21/2014 10:01 AM, Sebastian Huber wrote: >>>> >>>> >>>> Hello Gedare, >>>> >>>> thanks for your review. >>>> >>>> On 2014-05-20 16:58, Gedare Bloom wrote: >>>>>> >>>>>> >>>>>> @@ -54,7 +58,15 @@ SYM(_CPU_Context_switch): >>>>>>> >>>>>>> ld [%g6 + SPARC_PER_CPU_ISR_DISPATCH_DISABLE], %o4 >>>>>>> ! save it a bit later so we do not waste a couple of cycles >>>>>>> >>>>>>> +#ifdef RTEMS_PARAVIRT_XTRATUM >>>>>>> + mov %o0, %l6 >>>>>>> + mov sparc_get_psr_nr, %o0 >>>>>>> + __XM_AHC >>>>> >>>>> >>>>> I'm guessing these symbols are defined in xm.h? >>>>> >>>>>>> + mov %o0, %o2 >>>>>>> + mov %l6, %o0 >>>>>>> +#else /* RTEMS_PARAVIRT_XTRATUM */ >>>>>>> rd %psr, %o2 >>>>>>> +#endif /* RTEMS_PARAVIRT_XTRATUM */ >>>>> >>>>> >>>>> It would be cleaner code if you can write this get_psr as an assembler >>>>> macro, e.g. >>>>> >>>>> .macro GET_PSR REG, TMP0 >>>>> #ifdef RTEMS_PARAVIRT_XTRATUM >>>>> mov %o0, \TMP >>>>> mov sparc_get_psr_nr, %o0 >>>>> __XM_AHC >>>>> mov %o0, \REG >>>>> mov \TMP, %o0 >>>>> #else >>>>> rd %psr, \REG >>>>> #endif >>>>> >>>>> Same for some of the other functions that get used multiple times in >>>>> this file, such as set_psr, set_pil, and clear_pil. Putting these in a >>>>> header file would make them useful in start.S also. >>>>> >>>>> [...] >>>>> >>>> >>>> I also think that assembler macros could be useful here. >>>> >>>> Daniel, what is you opinion with respect to the usage of assembler >>>> macros >>>> in this file? >>> >>> >>> >>> Personally I prefer C preprocessor macros/defines mixed with the ASM code >>> instead. If the RTEMS project uses ASM macros I'm fine with it. >>> >> I noticed there was a macro in that file already, so I guess there is >> precedence for using them. Possibly this issue needs to be taken up at >> a higher level as a coding convention practice, as to whether to avoid >> ASM macros or not. I can see reasons for and against. > > > This file is currently free of assembler macros (the one you probably have > in mind disappeared with the commit > 7c0bd74c87b141454ae17ee1cfeeba42dc4b0df2). > > The PowerPC heavily uses assembler macros in the exceptions support. The C > pre-processor can be a bit tricky with multi-line statements since it > produces a one-liner. I don't have a strong opinion for using assembler > macros or not. > OK, the use of #ifdef..#endif is fine with me also. It can be left as-is since Daniel prefers the CPP approach at least for this particular case.
> > -- > Sebastian Huber, embedded brains GmbH > > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.hu...@embedded-brains.de > PGP : Public key available on request. > > Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. _______________________________________________ rtems-devel mailing list rtems-devel@rtems.org http://www.rtems.org/mailman/listinfo/rtems-devel
