Comment 11 for bug 1722849

I have had the questionable pleasure to implement or correct macros for critical sections on several target platforms, all with the help of GCC. My thoughts on the information collected in this issue so far:

I have always thought that volatile asm statements are as volatile as volatile memory accesses. If that is true, the original problem with GCC reordering volatile asm statements relative to an external function call which (without the compiler's knowledge) could hold volatile memory accesses is a bug.

Regarding the summary by mirosamek in #8, I would really recommend using "memory" as clobbering instead of only the primask_ variable. This is important since otherwise the values of non-volatile variables sampled outside of the critical section might be cached in registers and reused inside the critical section. This is probably not something that the coder of this particular code is aware of. If the critical section is used to keep consistency while reading/writing several global variables, this might be fatal.

I also see no reason to break up assembly code which must be executed in sequence, but that is more a matter of taste I guess.

Suggestion, given the general functionality of the previous macros:

/* critical section (save and restore interrupt status) */
#define CRIT_ENTRY(primask_) do { \
    __asm volatile ("mrs %0,PRIMASK \n" \
                    "cpsid i" \
                    : "=r" (primask_) :: "memory" ); \
} while (0)

#define CRIT_EXIT(primask_) \
    __asm volatile ("msr PRIMASK,%0" :: "r" (primask_) : "memory" )

The "memory" in CRIT_ENTRY is to avoid cached variables from before the critical section to be used within the critical section. The "memory" in CRIT_EXIT is to avoid any code which needs protection to be moved outside after the critical section.