[riot-notifications] [RIOT-OS/RIOT] cpu/arm7_common: Make irq_*() compiler barriers (#11440)

Marian Buschsieweke notifications at github.com
Thu Apr 25 11:08:03 CEST 2019


maribu commented on this pull request.



>      return retval;
 }
 
 int irq_is_in(void)
 {
     int retval;
-    __asm__ volatile(" mrs  %0, cpsr" : "=r"(retval) : /* no inputs */);
+    __asm__ volatile(" mrs  %0, cpsr" : "=r"(retval) : /* no inputs */ : "memory");

> Uff, is the compiler allowed to do this?

If a transformation does not change visible behavior from the compilers point of view, it is legal. The compiler is unaware of concurrent memory accesses. So speculative stores to `x` in the example above would be totally valid from the compilers viewpoint (even if `x` is a global), as the compiler does not take into account that other threads could read `x` at that very instance when the speculative store was done and not fixed subsequently.

> Do you have a reference?

This article. https://lwn.net/Articles/586838/ (If you want to fast-forward to relevant part search for speculative stores

In more detail: If the inline assembly in `irq_is_in()` is only marked as `volatile`, but no (compiler) barrier is added. Thus, non-`volatile` accesses can be reordered with regard to `irq_is_in()`. The store `x = 3` is neither `volatile` nor guarded by a memory barrier, so it could be moved up. If that store was done incorrectly, the subsequent `x = 7` will fix it. So in a single-threaded context this transformation will work just fine - which is all the compiler needs to prove.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11440#discussion_r278460285
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190425/d5f21cfc/attachment.html>


More information about the notifications mailing list